Conversation
There was a problem hiding this comment.
May main concern is that will removing this option affecting users to upgrade their running streaming pipeline to new version (even if they did not refer to related pipeline option)? Because for now DataflowPipelineOptions extends CloudDebuggerOptions.
CHANGES.md
Outdated
There was a problem hiding this comment.
2.47.0? we might need to wait for 2.48.0 or even 2.49.0
There was a problem hiding this comment.
in this case removed milestone and adjusted priority.
Note that the service is scheduled to shutdown on May 31 which is between the release of 2.47.0 (end Apr) and 2.48.0 (mid June), which means it would be removed in no later than v2.48.0.
There was a problem hiding this comment.
My vote would be to release a couple versions where the option explicitly throws a nice error explaining why it no longer works. Regardless, I agree that specifying the specific release it will be removed in would be nice - I'd vote we say 2.48.0
There was a problem hiding this comment.
2.47.0: add warning message about removing this from 2.48.0
2.48.0: remove it
Is this ok?
Codecov Report
@@ Coverage Diff @@
## master #25960 +/- ##
==========================================
- Coverage 81.07% 81.06% -0.02%
==========================================
Files 469 469
Lines 67165 67165
==========================================
- Hits 54453 54444 -9
- Misses 12712 12721 +9
Flags with carried forward coverage won't be shown. Click here to find out more. see 11 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Assigning reviewers. If you would like to opt out of this review, comment R: @damccorm for label build. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
damccorm
left a comment
There was a problem hiding this comment.
Mostly LGTM, thanks - other than my comment in CHANGES.md, could you also please update the description to be part of #25959 instead of fixes #25959 - we want the issue to stay open until we fully remove support
CHANGES.md
Outdated
There was a problem hiding this comment.
My vote would be to release a couple versions where the option explicitly throws a nice error explaining why it no longer works. Regardless, I agree that specifying the specific release it will be removed in would be nice - I'd vote we say 2.48.0
|
Reminder, please take a look at this pr: @damccorm |
|
waiting on author |
* Remove cloud debugger pipeline option * Eliminated cloud-debugger api dependency
|
R: @damccorm PTAL |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
|
Run Java_GCP_IO_Direct PreCommit |
|
Run Java_PVR_Flink_Batch PreCommit |
|
Run Java_IOs_Direct PreCommit |
|
Pending tests are passed not get updated to GitHub UI. PVR_Flink_Batch failure unrelated. Merging for now. |
Fixes #25959
Please add a meaningful description for your change here
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.mdwith 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.