-
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
[Avro] Deprecate Avro API classes in "sdks/java/core" #25534
[Avro] Deprecate Avro API classes in "sdks/java/core" #25534
Conversation
R: @mosche |
106678a
to
a1e1245
Compare
CC: @Abacn |
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
@@ -107,10 +107,14 @@ | |||
* org.apache.beam.sdk.transforms.GroupByKey} operations. | |||
* | |||
* @param <T> the type of elements handled by this coder | |||
* @deprecated Avro classes are deprecated to use in module <code>beam-sdks-java-core</code> and |
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.
Nitpick... maybe like below? Though not important
* @deprecated Avro classes are deprecated to use in module <code>beam-sdks-java-core</code> and | |
* @deprecated Avro related classes are deprecated in module <code>beam-sdks-java-core</code> and |
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.
Please also mention the new import that should be used instead to make the migration as easy as possible. Same for all below...
CHANGES.md
Outdated
@@ -88,6 +88,7 @@ | |||
## Deprecations | |||
|
|||
* X behavior is deprecated and will be removed in X versions ([#X](https://github.com/apache/beam/issues/X)). | |||
* Avro classes are deprecated to use in module `beam-sdks-java-core` and will be eventually removed ([#24749](https://github.com/apache/beam/issues/24749)). |
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.
Nitpick... maybe like below? Though not important
* Avro classes are deprecated to use in module `beam-sdks-java-core` and will be eventually removed ([#24749](https://github.com/apache/beam/issues/24749)). | |
* Avro related classes are deprecated in module `beam-sdks-java-core` and will be eventually removed ([#24749](https://github.com/apache/beam/issues/24749)). |
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.
It would be good to add a short note how to migrate, e.g. mentioning the new module and package name to use.
a1e1245
to
b65b0a4
Compare
@mosche Thanks for review, I addressed your comments, ptal |
b65b0a4
to
304034b
Compare
retest this please |
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.
Thanks for adding the migration notes @aromanenko-dev 💯
retest this please |
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.
Thanks
Assigning reviewers. If you would like to opt out of this review, comment R: @apilloud for label java. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
304034b
to
42827ac
Compare
Failed checks are not related to this change |
[!!!] It must be merged ONLY AFTER #24992 is merged. [!!!]
All Beam modules are migrated to use
sdks/java/extensions/avro
. So it's needed to deprecate Avro API classes insdks/java/core
but keep them for a while for a sake of backward compatibility.Affected classes:
Fixes #24749
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.