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: implements revocation in DataPlaneAuthorizationService #4019

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

wolf4ood
Copy link
Contributor

@wolf4ood wolf4ood commented Mar 18, 2024

What this PR changes/adds

implements revocation in DataPlaneAuthorizationService

Why it does that

Notify the DataPlaneAuthorizationService that the transfer has been terminated or suspended

Further notes

List other areas of code that have changed but are not necessarily linked to the main feature. This could be method
signature changes, package declarations, bugs that were encountered and were fixed inline, etc.

Linked Issue(s)

Closes #3997

Please be sure to take a look at the contributing guidelines and our etiquette for pull requests.

@wolf4ood wolf4ood force-pushed the feat/3997_terminate_mesage branch 2 times, most recently from 2779aa6 to 2eddb30 Compare March 18, 2024 16:12
@wolf4ood wolf4ood self-assigned this Mar 18, 2024
@wolf4ood wolf4ood added the enhancement New feature or request label Mar 18, 2024
@codecov-commenter
Copy link

codecov-commenter commented Mar 18, 2024

Codecov Report

Attention: Patch coverage is 88.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 73.63%. Comparing base (7f20ba5) to head (f5e48ec).
Report is 154 commits behind head on main.

Files Patch % Lines
...ransfer/listener/TransferProcessEventListener.java 0.00% 2 Missing ⚠️
.../eclipse/edc/connector/dataplane/spi/DataFlow.java 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4019      +/-   ##
==========================================
+ Coverage   71.74%   73.63%   +1.88%     
==========================================
  Files         919      984      +65     
  Lines       18457    19896    +1439     
  Branches     1037     1119      +82     
==========================================
+ Hits        13242    14650    +1408     
- Misses       4756     4771      +15     
- Partials      459      475      +16     

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

@wolf4ood wolf4ood force-pushed the feat/3997_terminate_mesage branch 2 times, most recently from 3ac573f to 029c883 Compare March 18, 2024 18:06
@wolf4ood wolf4ood marked this pull request as ready for review March 18, 2024 18:10
* @param reason The reason for the revocation
* @return Success if revoked, failure otherwise
*/
Result<Void> revoke(String transferProcessId, String reason);

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'reason' is never used.
@wolf4ood wolf4ood merged commit 964e40b into eclipse-edc:main Mar 19, 2024
16 checks passed
@wolf4ood wolf4ood deleted the feat/3997_terminate_mesage branch March 19, 2024 15:13
* @param reason The reason for the revocation
* @return Success if revoked, failure otherwise
*/
Result<Void> revoke(String transferProcessId, String reason);
Copy link
Member

Choose a reason for hiding this comment

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

is the reason needed here? It makes sense in the TransferProcess context because the counter part then knows why, but I'm not sure the data plane needs to know. In fact in the implementation is not used

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For our internal impl we don't use it, but that might be useful for other impl? that's why i left it

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

Successfully merging this pull request may close these issues.

DPS: add support for terminate message
4 participants