-
Notifications
You must be signed in to change notification settings - Fork 223
Bringing Durable Task Java as a Maven module inside the Java SDK #1575
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
Conversation
8e649ef to
4e9e636
Compare
|
Merging this PR will mean that we need to archive https://github.com/dapr/durabletask-java/ |
af11647 to
5b6f098
Compare
1c8269e to
8e842e8
Compare
|
@mcruzdev @artur-ciocanu folks I want you to be aware of this, as the PRs to align versions and remove netty-shaded are all collapsed here. |
|
@dapr/approvers-java-sdk @dapr/maintainers-java-sdk please review and provide feedback. We should not merge other PRs in durabletask-java until this is merged here to avoid divergence. For now, we will need to leave with low code coverage, as we are brining this module to the SDK, it will be easier to add tests as we move forward. |
| @@ -0,0 +1,342 @@ | |||
| ///* | |||
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.
If all the other changes are approved I am happy to fix this tests as this follows the same pattern as this PR: #1529 were we remove all these tests and related dependencies in favour of pre made certs. Please leave this file out of the overall review of this PR. 🙏
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.
Issue created that explains the migration needed for this test, please never enable this test as is again: #1604
|
@salaboy this PR LGTM. Should we exclude this module from the coverage ? Otherwise codecov will start failing for every PR. |
Yes, that is something we can do. I really want us to bring the testing level up for this module. |
cicoyle
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.
I believe we need to add to our release automation script if we proceed with this PR. Specifically we will need to add to the update_sdk_version.sh something like the following:
mvn versions:set -DnewVersion=$DAPR_JAVA_SDK_VERSION -f durabletask-client/pom.xml
Also, could you verify if we need changes in the update_docs.sh script for this new module, as I believe we will need changes there.
Signed-off-by: salaboy <[email protected]>
Signed-off-by: Matheus Cruz <[email protected]> Signed-off-by: salaboy <[email protected]>
* Align Java API with other languages Signed-off-by: Matheus Cruz <[email protected]> * Update documentation Signed-off-by: Matheus Cruz <[email protected]> * Change return type of waitForWorkflowStart method Signed-off-by: artur-ciocanu <[email protected]> --------- Signed-off-by: Matheus Cruz <[email protected]> Signed-off-by: artur-ciocanu <[email protected]> Co-authored-by: artur-ciocanu <[email protected]> Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
* Force Jackson version to override the SB Jackson version Signed-off-by: Artur Ciocanu <[email protected]> * Move all the Jackson deps to parent POM. Signed-off-by: Artur Ciocanu <[email protected]> * Ensure app JAR build order Signed-off-by: Artur Ciocanu <[email protected]> * Remove explicit Jackson from sdk-tests module. Signed-off-by: Artur Ciocanu <[email protected]> * Make sure <scope>test</scope> is used for test dependencies. Signed-off-by: Artur Ciocanu <[email protected]> * Remove extra Jackson modules. Signed-off-by: Artur Ciocanu <[email protected]> --------- Signed-off-by: Artur Ciocanu <[email protected]> Signed-off-by: salaboy <[email protected]>
* Preview New README * Preview New README 2 * Preview New README 3 * docs: add architecture diagram showing Java SDK interaction with Dapr runtime (close #<915>) * docs: add architecture diagram showing Java SDK interaction with Dapr runtime CORRECTION (close #<915>) * docs: add architecture diagram showing Java SDK interaction with Dapr runtime (close #<915>) * docs: add architecture diagram showing Java SDK interaction with Dapr runtime (close #<915>) --------- Co-authored-by: Siri Varma Vegiraju <[email protected]> Co-authored-by: artur-ciocanu <[email protected]> Co-authored-by: Cassie Coyle <[email protected]> Signed-off-by: salaboy <[email protected]>
* Add statestore example with Outbox pattern Signed-off-by: Matheus Cruz <[email protected]> * Clean events after each test Signed-off-by: Matheus Cruz <[email protected]> * Add license header Signed-off-by: Matheus Cruz <[email protected]> * Apply pull request suggestions Signed-off-by: Matheus Cruz <[email protected]> --------- Signed-off-by: Matheus Cruz <[email protected]> Co-authored-by: salaboy <[email protected]> Signed-off-by: salaboy <[email protected]>
* adding new method signature plus test Signed-off-by: salaboy <[email protected]> * re adding imports Signed-off-by: salaboy <[email protected]> * fixing style Signed-off-by: salaboy <[email protected]> * checking empty metadata Signed-off-by: salaboy <[email protected]> * copy meta for safety and check if key is present Signed-off-by: salaboy <[email protected]> * Centralize Maven dependency version management (dapr#1564) Signed-off-by: salaboy <[email protected]> * Fix dependencies multi app build and add proper test deps (dapr#1572) * Force Jackson version to override the SB Jackson version Signed-off-by: Artur Ciocanu <[email protected]> * Move all the Jackson deps to parent POM. Signed-off-by: Artur Ciocanu <[email protected]> * Ensure app JAR build order Signed-off-by: Artur Ciocanu <[email protected]> * Remove explicit Jackson from sdk-tests module. Signed-off-by: Artur Ciocanu <[email protected]> * Make sure <scope>test</scope> is used for test dependencies. Signed-off-by: Artur Ciocanu <[email protected]> * Remove extra Jackson modules. Signed-off-by: Artur Ciocanu <[email protected]> --------- Signed-off-by: Artur Ciocanu <[email protected]> Signed-off-by: salaboy <[email protected]> * reverting pom Signed-off-by: salaboy <[email protected]> * fix codestyle Signed-off-by: salaboy <[email protected]> * using metaCopy Signed-off-by: salaboy <[email protected]> --------- Signed-off-by: salaboy <[email protected]> Signed-off-by: Artur Ciocanu <[email protected]> Co-authored-by: artur-ciocanu <[email protected]> Signed-off-by: salaboy <[email protected]>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 5. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v4...v5) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: salaboy <[email protected]>
* Add gRPC support to Dapr testcontainer Signed-off-by: wlfgang <[email protected]> * Avoid using null to indicate default value Signed-off-by: wlfgang <[email protected]> --------- Signed-off-by: wlfgang <[email protected]> Co-authored-by: artur-ciocanu <[email protected]> Co-authored-by: wlfgang <[email protected]> Signed-off-by: salaboy <[email protected]>
Signed-off-by: Artur Ciocanu <[email protected]> Signed-off-by: salaboy <[email protected]>
…apr#1589) * example Signed-off-by: Cassandra Coyle <[email protected]> * docs for example Signed-off-by: Cassandra Coyle <[email protected]> --------- Signed-off-by: Cassandra Coyle <[email protected]> Signed-off-by: salaboy <[email protected]>
…r#1596) Signed-off-by: salaboy <[email protected]>
* Adding a Flux based subscribeToEvents method Signed-off-by: Artur Ciocanu <[email protected]> * Simplify GRPC stream handling Signed-off-by: Artur Ciocanu <[email protected]> * Simplify Javadoc Signed-off-by: Artur Ciocanu <[email protected]> * Fix unit tests and simplify implementation Signed-off-by: Artur Ciocanu <[email protected]> * Adding event subscriber stream observer to simplify subscription logic Signed-off-by: Artur Ciocanu <[email protected]> * Use start() method to start stream subscription Signed-off-by: Artur Ciocanu <[email protected]> * Add unit test for event suscriber observer Signed-off-by: Artur Ciocanu <[email protected]> * Improve the tests a little bit Signed-off-by: Artur Ciocanu <[email protected]> * Remove the unnecessary method Signed-off-by: Artur Ciocanu <[email protected]> * Improve error handling and use CloudEvent wrapper Signed-off-by: Artur Ciocanu <[email protected]> * Fix unit tests asserts Signed-off-by: Artur Ciocanu <[email protected]> * Adjust Java examples for Subscriber Signed-off-by: Artur Ciocanu <[email protected]> --------- Signed-off-by: Artur Ciocanu <[email protected]> Signed-off-by: salaboy <[email protected]>
* Remove SDK docs due to migration to main Docs repo Signed-off-by: Marc Duiker <[email protected]> * Remove sed lines related to sdk docs Signed-off-by: Marc Duiker <[email protected]> --------- Signed-off-by: Marc Duiker <[email protected]> Co-authored-by: salaboy <[email protected]> Signed-off-by: salaboy <[email protected]>
Signed-off-by: salaboy <[email protected]>
3cc538f to
f22d0bd
Compare
Signed-off-by: salaboy <[email protected]>
I've clean up the script to be simpler to maintain, https://github.com/dapr/java-sdk/pull/1575/files#diff-e0e9e1d7c713596b37959c8e4a8f9f5540b77c1a741e31e8c24d5243f35f113d It was getting confusing and there is really no need to change that for child modules, now that we don't have any alpha modules in the SDK. On the docs script, I don't think we need a change. |
Instaed of excluding this from codecov, what we should do is to set the current coverage number as the base for that moduel, and then improve from there. @siri-varma does tha make sense? |
cicoyle
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 Mauricio! mostly lgtm, question on an unrelated change I noticed (and commented about) and can you create the issue in the java sdk to add back in the test that was commented out, so we dont forget about it?
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1575 +/- ##
============================================
+ Coverage 76.91% 78.67% +1.76%
- Complexity 1592 1957 +365
============================================
Files 145 217 +72
Lines 4843 5970 +1127
Branches 562 661 +99
============================================
+ Hits 3725 4697 +972
- Misses 821 932 +111
- Partials 297 341 +44 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: #1571
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list: