-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[Java] Managed API #30808
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
[Java] Managed API #30808
Conversation
chamikaramj
left a comment
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. This is pretty close to what we want IMO.
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/ManagedSchemaTransformProvider.java
Outdated
Show resolved
Hide resolved
...a/io/kafka/src/main/java/org/apache/beam/sdk/io/kafka/KafkaWriteSchemaTransformProvider.java
Outdated
Show resolved
Hide resolved
|
Let's also add unit-testing using existing SchemaTransforms (we'll have to mock/override the set of transforms for a group or the pattern used to select the set of transforms for a group). |
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Show resolved
Hide resolved
|
Checks are failing. Will not request review until checks are succeeding. If you'd like to override that behavior, comment |
chamikaramj
left a comment
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!
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/ManagedSchemaTransformProvider.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/test/java/org/apache/beam/sdk/managed/ManagedTest.java
Show resolved
Hide resolved
|
Also, please fix spotless/lint failures. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #30808 +/- ##
============================================
- Coverage 61.25% 60.34% -0.92%
+ Complexity 4470 2987 -1483
============================================
Files 853 659 -194
Lines 74090 65994 -8096
Branches 4307 3231 -1076
============================================
- Hits 45385 39823 -5562
+ Misses 25224 23078 -2146
+ Partials 3481 3093 -388 ☔ View full report in Codecov by Sentry. |
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
chamikaramj
left a comment
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. LGTM.
This looks great!
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
sdks/java/managed/src/main/java/org/apache/beam/sdk/managed/Managed.java
Outdated
Show resolved
Hide resolved
chamikaramj
left a comment
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. LGTM.
Managed transforms API for Java
Fixes #30830