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

feat: add java client #20

Merged
merged 5 commits into from
Nov 13, 2024
Merged

feat: add java client #20

merged 5 commits into from
Nov 13, 2024

Conversation

strieflin
Copy link
Member

No description provided.

@strieflin strieflin force-pushed the add-java-client branch 2 times, most recently from 490ace3 to 434a37c Compare November 12, 2024 13:41
@strieflin strieflin marked this pull request as ready for review November 12, 2024 15:21
@strieflin strieflin requested a review from a team as a code owner November 12, 2024 15:21
Copy link
Member

@sbckr sbckr left a comment

Choose a reason for hiding this comment

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

handling policy id prefix stackable/bundles/ seems inconsistent and inconvenient. Why not leaving it to the service at all?

if (index == 0) {
commonPolicies.addAll(result._2.get());
} else {
commonPolicies.retainAll(result._2.get());
Copy link
Member

Choose a reason for hiding this comment

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

It is ignored if at least one provider has policies defined that are not present on other(s).
Should that be logged somehow? Is this considered okay?
It could be a common scenario if policies are updated (added or deleted) and changes are not yet reflected on all VCPs. However, an option to have this information printed allows debugging if policies are not enrolled properly

Copy link
Member Author

Choose a reason for hiding this comment

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

This is now logged on debug level.

Copy link

codecov bot commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 34 lines in your changes missing coverage. Please review.

Project coverage is 80.00%. Comparing base (8284e69) to head (46c625f).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...ava/io/carbynestack/thymus/client/ThymusError.java 38.88% 11 Missing ⚠️
...io/carbynestack/thymus/client/ThymusVCPClient.java 83.92% 8 Missing and 1 partial ⚠️
.../io/carbynestack/thymus/client/PolicyResponse.java 62.50% 5 Missing and 1 partial ⚠️
.../io/carbynestack/thymus/client/ThymusEndpoint.java 69.23% 1 Missing and 3 partials ⚠️
.../io/carbynestack/thymus/client/NamespacedName.java 66.66% 1 Missing and 2 partials ⚠️
...ain/java/io/carbynestack/thymus/client/Policy.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master      #20       +/-   ##
=============================================
+ Coverage     61.65%   80.00%   +18.34%     
- Complexity        0       47       +47     
=============================================
  Files             3        7        +4     
  Lines           133      170       +37     
  Branches          0       12       +12     
=============================================
+ Hits             82      136       +54     
+ Misses           41       26       -15     
+ Partials         10        8        -2     
Flag Coverage Δ
java-client 80.00% <80.00%> (?)
services ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
.../io/carbynestack/thymus/client/ThymusVCClient.java 100.00% <100.00%> (ø)
...ain/java/io/carbynestack/thymus/client/Policy.java 66.66% <66.66%> (ø)
.../io/carbynestack/thymus/client/NamespacedName.java 66.66% <66.66%> (ø)
.../io/carbynestack/thymus/client/ThymusEndpoint.java 69.23% <69.23%> (ø)
.../io/carbynestack/thymus/client/PolicyResponse.java 62.50% <62.50%> (ø)
...io/carbynestack/thymus/client/ThymusVCPClient.java 83.92% <83.92%> (ø)
...ava/io/carbynestack/thymus/client/ThymusError.java 38.88% <38.88%> (ø)

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8284e69...46c625f. Read the comment docs.

sbckr
sbckr previously approved these changes Nov 13, 2024
Copy link
Member

@sbckr sbckr left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@sbckr sbckr left a comment

Choose a reason for hiding this comment

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

LGTM

@strieflin strieflin merged commit 8766215 into master Nov 13, 2024
9 checks passed
@strieflin strieflin deleted the add-java-client branch November 13, 2024 10:23
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.

2 participants