Skip to content
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

Adding metadata endpoint #1049

Merged
merged 17 commits into from
Jun 25, 2024
Merged

Adding metadata endpoint #1049

merged 17 commits into from
Jun 25, 2024

Conversation

salaboy
Copy link
Contributor

@salaboy salaboy commented Jun 10, 2024

Description

Adding metadata endpoint on HTTP and gRPC clients. Please review and advice.

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: #1048

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@salaboy salaboy requested review from a team as code owners June 10, 2024 15:55
@salaboy salaboy marked this pull request as draft June 10, 2024 15:55
@salaboy salaboy marked this pull request as ready for review June 11, 2024 09:58
@salaboy salaboy changed the title [WIP] Adding metadata endpoint Adding metadata endpoint Jun 11, 2024
@salaboy
Copy link
Contributor Author

salaboy commented Jun 11, 2024

@artursouza there seems to be a flaky test with bindings and docker compose, but the code seems to be working as expected.

Copy link
Contributor

@artur-ciocanu artur-ciocanu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@salaboy I really like how you put everything together. I have left a few tiny comments.

sdk/src/main/java/io/dapr/client/DaprClientGrpc.java Outdated Show resolved Hide resolved
sdk/src/main/java/io/dapr/client/DaprClientGrpc.java Outdated Show resolved Hide resolved
sdk/src/main/java/io/dapr/client/DaprClientHttp.java Outdated Show resolved Hide resolved
sdk/src/main/java/io/dapr/client/DaprClientHttp.java Outdated Show resolved Hide resolved
@artur-ciocanu
Copy link
Contributor

@artursouza I have forked the repo here: https://github.com/artur-ciocanu/java-sdk. I tried to run the Auto Validate Examples GitHub Workflow, here is the run for master: https://github.com/artur-ciocanu/java-sdk/actions/runs/9582102678/job/26420345709 and I have run into similar issues as current PR.

I was wondering if there is a way to run some of the workflow failing steps in isolation to figure out what should be adjusted to make it more robust.

Thanks for your help!

@cicoyle
Copy link
Contributor

cicoyle commented Jun 19, 2024

@artursouza I have forked the repo here: https://github.com/artur-ciocanu/java-sdk. I tried to run the Auto Validate Examples GitHub Workflow, here is the run for master: https://github.com/artur-ciocanu/java-sdk/actions/runs/9582102678/job/26420345709 and I have run into similar issues as current PR.

I was wondering if there is a way to run some of the workflow failing steps in isolation to figure out what should be adjusted to make it more robust.

Thanks for your help!

Hey @artur-ciocanu - Thanks for the review. You should be able to run what CI is running.

So go to the examples directory and run the examples locally. It looks like its this test that is failing - see the README for instructions here

I see Could not update sidecar metadata for appPID: PUT http://127.0.0.1:36963/v1.0/metadata/appPID giving up after 5 attempt(s): Put "http://127.0.0.1:36963/v1.0/metadata/appPID": dial tcp 127.0.0.1:36963: connect: connection refused -> is appPID correct? I also see Flag --components-path has been deprecated which we might need to update to not use that to get it passing.

Copy link
Contributor

@cicoyle cicoyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

few nits and a question, but generally the code looks good. Thanks @salaboy 🎉

@artur-ciocanu
Copy link
Contributor

@salaboy for the Auto Validate Examples issue I have opened a PR: #1054. Please take a look.

@salaboy salaboy force-pushed the metadata-1048 branch 2 times, most recently from 6ae59cb to f6c5f27 Compare June 21, 2024 15:04
@salaboy
Copy link
Contributor Author

salaboy commented Jun 21, 2024

@cicoyle @artursouza I have no idea why this is failing now. I can see this failing -> DaprClientGrpcTelemetryTest but the code in this PR hasn't changed anything there.

@salaboy salaboy force-pushed the metadata-1048 branch 2 times, most recently from 2101fdf to 732e7f1 Compare June 24, 2024 09:10
salaboy added 3 commits June 24, 2024 10:23
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
salaboy added 14 commits June 24, 2024 10:23
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Signed-off-by: salaboy <Salaboy@gmail.com>
Copy link

codecov bot commented Jun 24, 2024

Codecov Report

Attention: Patch coverage is 63.06306% with 41 lines in your changes missing coverage. Please review.

Project coverage is 77.46%. Comparing base (d759c53) to head (837be4a).
Report is 21 commits behind head on master.

Current head 837be4a differs from pull request most recent head e594de5

Please upload reports for the commit e594de5 to get more accurate results.

Files Patch % Lines
.../main/java/io/dapr/client/domain/DaprMetadata.java 50.00% 4 Missing and 7 partials ⚠️
...va/io/dapr/client/domain/SubscriptionMetadata.java 54.54% 4 Missing and 6 partials ⚠️
.../java/io/dapr/client/domain/ComponentMetadata.java 55.00% 4 Missing and 5 partials ⚠️
.../main/java/io/dapr/client/domain/RuleMetadata.java 53.33% 4 Missing and 3 partials ⚠️
...k/src/main/java/io/dapr/client/DaprClientGrpc.java 91.66% 2 Missing ⚠️
...k/src/main/java/io/dapr/client/DaprClientHttp.java 71.42% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1049      +/-   ##
============================================
+ Coverage     76.91%   77.46%   +0.55%     
- Complexity     1592     1607      +15     
============================================
  Files           145      148       +3     
  Lines          4843     4908      +65     
  Branches        562      583      +21     
============================================
+ Hits           3725     3802      +77     
+ Misses          821      800      -21     
- Partials        297      306       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@salaboy salaboy requested review from artursouza and cicoyle June 24, 2024 12:11
@artursouza artursouza merged commit 7e09841 into dapr:master Jun 25, 2024
10 checks passed
@artursouza
Copy link
Member

@artursouza I have forked the repo here: https://github.com/artur-ciocanu/java-sdk. I tried to run the Auto Validate Examples GitHub Workflow, here is the run for master: https://github.com/artur-ciocanu/java-sdk/actions/runs/9582102678/job/26420345709 and I have run into similar issues as current PR.

I was wondering if there is a way to run some of the workflow failing steps in isolation to figure out what should be adjusted to make it more robust.

Thanks for your help!

You can run each step of the workflow manually, including the mechanical markdown script. It seems that the timeout increase did the trick.

@cicoyle cicoyle added this to the v1.12 milestone Jul 23, 2024
@marcduiker
Copy link
Contributor

@holopin-bot @salaboy Thanks!

Copy link

holopin-bot bot commented Aug 15, 2024

Congratulations @salaboy, the maintainer of this repository has issued you a badge! Here it is: https://holopin.io/claim/clzva9l2k06280dlfe716ycjb

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

metadata API are not exposed in the Java SDK
5 participants