Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Reporting] code cleanup for reporting browser build/install/setup utilities #98799

Merged
merged 12 commits into from
Apr 30, 2021

Conversation

tsullivan
Copy link
Member

@tsullivan tsullivan commented Apr 29, 2021

The main goals are to convert files to TypeScript, and to improve the Python build scripts. Some of the changes simply convert code to TypeScript.

  • x-pack/build_chromium/*: this area of code is responsible for creating the builds
  • x-pack/plugins/reporting/server/browsers: this area of code is responsible for downloading, unzipping, and copying the builds

Precedes #98688

@tsullivan tsullivan force-pushed the reporting/browser-install-cleanup branch 3 times, most recently from 73a0e00 to 9b502d3 Compare April 29, 2021 17:44
@tsullivan tsullivan force-pushed the reporting/browser-install-cleanup branch from 6843b65 to 0632ba1 Compare April 29, 2021 17:47
@@ -136,3 +145,6 @@ def archive_file(name):
print('Creating ' + path.join(src_path, md5_filename))
with open (md5_filename, 'w') as f:
f.write(md5_file(zip_filename))

runcmd('gsutil cp ' + path.join(src_path, zip_filename) + ' gs://headless_shell_staging')
runcmd('gsutil cp ' + path.join(src_path, md5_filename) + ' gs://headless_shell_staging')
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @spalger : this adds an auto-upload to a staging bucket as we talked about.

I think this suffices the problem we had here: #98688 (comment)

This is without adding a dry-run option to the script, as discussed. LMK if you think it needs the dry-run option. (It would be a pain since a run takes hours.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think this is the best approach, then staging assets can just be gsutil mvd to the headless_shell bucket when they're verified.

Might be nice to use an environment variable to switch out the URLs to actually download from staging so we can test things before promoting them.

use_libpci = false
use_pulseaudio = false
use_udev = false

is_debug = false
symbol_level = 0
is_component_build = false
remove_webcore_debug_symbols = true
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove_webcore_debug_symbols caused warning logs during compilation: it is unused

@tsullivan tsullivan changed the title [Reporting] code cleanup for reporting browser setup utilities [Reporting] code cleanup for reporting browser build/install/setup utilities Apr 29, 2021
archivesPath: path.resolve(__dirname, '../../../../../../.chromium'),
baseUrl: 'https://storage.googleapis.com/headless_shell/',
packages: [
// We download zip files from a Kibana team GCS bucket named `headless_shell`
Copy link
Member Author

@tsullivan tsullivan Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was almost entirely re-written because there is a lot of code that uses the object, and those contexts had no abstractions available to work with the structure of the data.


import { readableEnd } from './util';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

util was removed because it didn't contain anything significant and the code inside is easy enough to define in-line where it is needed.

@tsullivan tsullivan force-pushed the reporting/browser-install-cleanup branch from 8b73fe0 to ceeb4d9 Compare April 29, 2021 18:24
@tsullivan tsullivan requested review from dokmic, tylersmalley and a team April 29, 2021 18:25
@tsullivan tsullivan added release_note:skip Skip the PR/issue when compiling release notes v7.14.0 v8.0.0 technical debt Improvement of the software architecture and operational architecture (Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead Team:AppServices labels Apr 29, 2021
f.close()
argsgn_file_out = path.abspath('out/headless/args.gn')
runcmd('cp ' + argsgn_file + ' ' + argsgn_file_out)
runcmd('echo \'target_cpu="' + arch_name + '"\' >> ' + argsgn_file_out)
Copy link
Member Author

@tsullivan tsullivan Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the f.write code was not working most of the time: seemed to be a race condition where the previous line of code would complete later than expected

remove_webcore_debug_symbols = true

# Please, consult @elastic/kibana-security before changing/removing this option.
use_kerberos = false
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved these lines down to match the order of lines with ../windows/args.gn

{
platforms: ['darwin', 'freebsd', 'openbsd'],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kibana doesn't support freebsd or openbsd

await del(path, { force: true });
}
});
await Promise.all(
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

asyncMap was not that useful as a helper

@tsullivan tsullivan marked this pull request as ready for review April 29, 2021 23:17
@tsullivan tsullivan requested review from a team as code owners April 29, 2021 23:17
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-reporting-services (Team:Reporting Services)

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Operations specific code LGTM

src/dev/chromium_version.ts Outdated Show resolved Hide resolved
@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@tsullivan
Copy link
Member Author

@elasticmachine merge upstream

@tsullivan tsullivan enabled auto-merge (squash) April 30, 2021 22:49
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@tsullivan tsullivan merged commit 832d3b7 into elastic:master Apr 30, 2021
@tsullivan tsullivan deleted the reporting/browser-install-cleanup branch April 30, 2021 23:27
tsullivan added a commit to tsullivan/kibana that referenced this pull request Apr 30, 2021
…ilities (elastic#98799)

* [Reporting] code cleanup for reporting browser setup utilities

* fix target_cpu

* Update README.md

* Update README.md

* add note about target_cpu

* Update paths.ts

* more cleanup

* Update src/dev/chromium_version.ts

Co-authored-by: Michael Dokolin <[email protected]>

* remove bug

Co-authored-by: Michael Dokolin <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	src/dev/code_coverage/ingest_coverage/__tests__/enumerate_patterns.test.js
#	src/dev/code_coverage/ingest_coverage/__tests__/mocks/team_assign_mock.txt
#	src/dev/code_coverage/ingest_coverage/__tests__/transforms.test.js
tsullivan added a commit that referenced this pull request May 2, 2021
…tup utilities (#98799) (#98998)

* [Reporting] code cleanup for reporting browser build/install/setup utilities (#98799)

* [Reporting] code cleanup for reporting browser setup utilities

* fix target_cpu

* Update README.md

* Update README.md

* add note about target_cpu

* Update paths.ts

* more cleanup

* Update src/dev/chromium_version.ts

Co-authored-by: Michael Dokolin <[email protected]>

* remove bug

Co-authored-by: Michael Dokolin <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
# Conflicts:
#	src/dev/code_coverage/ingest_coverage/__tests__/enumerate_patterns.test.js
#	src/dev/code_coverage/ingest_coverage/__tests__/mocks/team_assign_mock.txt
#	src/dev/code_coverage/ingest_coverage/__tests__/transforms.test.js

* fix bad merge

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
(Deprecated) Feature:Reporting Use Reporting:Screenshot, Reporting:CSV, or Reporting:Framework instead release_note:skip Skip the PR/issue when compiling release notes technical debt Improvement of the software architecture and operational architecture v7.14.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants