-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
[Python] Log dependencies installed in submission environment #28564
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #28564 +/- ##
==========================================
+ Coverage 38.23% 38.47% +0.24%
==========================================
Files 696 698 +2
Lines 101878 102520 +642
==========================================
+ Hits 38952 39449 +497
- Misses 61309 61439 +130
- Partials 1617 1632 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
Run Python_Integration PreCommit |
Some unit tests are failing because there is additional staging file now which will always be present. Got the solution. I'll update the PR. Defer review until then. |
Assigning reviewers. If you would like to opt out of this review, comment R: @jrmccluskey for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM mod a logging nit
R: @chamikaramj could you comment on the external transform environment. The external_transform environment tests would fail if there is an additional staging file by default. It complains about no artifact service when it tries to resolve that artifact. |
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
Can we stage job submission dependencies without including them in the runtime environment defintion? |
R: @tvalentyn this is ready for review Changes to note:
|
sdks/python/apache_beam/runners/dataflow/dataflow_runner_test.py
Outdated
Show resolved
Hide resolved
sdks/python/apache_beam/runners/portability/expansion_service.py
Outdated
Show resolved
Hide resolved
R: @tvalentyn Barring the lint failure, all tests pass. |
sdks/python/apache_beam/runners/dataflow/dataflow_runner_test.py
Outdated
Show resolved
Hide resolved
sdks/python/apache_beam/runners/dataflow/dataflow_runner_test.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Is this ready to merge? |
i left one comment, after that it should be ready to merge. |
Done, I'll merge once the check passes |
…#28564) * log runtime dependencies * log submission env dependencies * rm spare line * update tests for staged files * update equals test to ignore artifacts * handle unit tests, refactor * unit test sub env staging, convert to string * change Log to Printf * change log level to warning * try env * add artifact_service method * correct urn * fix artifact comparison, file reader * add mock for python sdk dependencies and update artifact service method * fix lint * use magic mock instead of mocking entire function * update dataflow runner test * Update sdks/python/apache_beam/runners/dataflow/dataflow_runner_test.py * use debug option to disable * remove tmp directory mock
Saves the submission environment dependencies and stage it. Logs it along with the runtime dependencies.
Fixes #28563
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123
), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>
instead.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.