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

refactor(api): move data plane select functionality to control api #4189

Merged

Conversation

ndr-brt
Copy link
Member

@ndr-brt ndr-brt commented May 16, 2024

What this PR changes/adds

Add select and getAll functionality to dataplane selector control-api, and deprecate select on the management-api

Why it does that

Separation of responsibilities.

Further notes

  • made all methods of DataPlaneSelectorService return ServiceResult, this makes the interaction of the Remote implementation more consistent, because there could be situations (unlike with the Embedded one) where the communication with the server fails.
  • removed destination from the dataplane selection request, because it is not really necessary (source and transferType are enough)
  • in the DataPlaneSignalingFlowController I left a TODO for a refactoring about the way the DataPlaneClient is created (in my opinion dataplaneselection should be encapsulated in the factory, so the condition "is data plane embedded" will be more explicit and testable

Linked Issue(s)

Closes #4177

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

@ndr-brt ndr-brt added refactoring Cleaning up code and dependencies api Feature related to the (REST) api labels May 16, 2024
@ndr-brt ndr-brt requested review from wolf4ood and bscholtes1A May 16, 2024 08:03
DataPlaneInstance select(DataAddress source, DataAddress destination, @Nullable String selectionStrategy, @Nullable String transferType);

@Deprecated(since = "0.6.4")
default DataPlaneInstance select(DataAddress source, DataAddress destination, @Nullable String selectionStrategy, @Nullable String transferType) {

Check notice

Code scanning / CodeQL

Useless parameter Note

The parameter 'destination' is never used.
@codecov-commenter
Copy link

codecov-commenter commented May 16, 2024

Codecov Report

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

Project coverage is 74.99%. Comparing base (7f20ba5) to head (32c46f8).
Report is 258 commits behind head on main.

Files Patch % Lines
...plane/selector/RemoteDataPlaneSelectorService.java 40.00% 17 Missing and 1 partial ⚠️
...java/org/eclipse/edc/spi/result/ServiceResult.java 28.57% 5 Missing ⚠️
...taplane/selector/spi/DataPlaneSelectorService.java 0.00% 4 Missing ⚠️
...ormer/JsonObjectToSelectionRequestTransformer.java 83.33% 1 Missing and 1 partial ⚠️
...ctor/service/EmbeddedDataPlaneSelectorService.java 83.33% 1 Missing ⚠️
.../java/org/eclipse/edc/api/model/ApiCoreSchema.java 0.00% 1 Missing ⚠️
...ector/control/api/DataplaneSelectorControlApi.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    #4189      +/-   ##
==========================================
+ Coverage   71.74%   74.99%   +3.25%     
==========================================
  Files         919     1015      +96     
  Lines       18457    20631    +2174     
  Branches     1037     1161     +124     
==========================================
+ Hits        13242    15473    +2231     
+ Misses       4756     4651     -105     
- Partials      459      507      +48     

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

@ndr-brt ndr-brt force-pushed the 4177-data-plane-selector-control-api branch from d7bad75 to 1044d45 Compare May 16, 2024 13:18
@ndr-brt ndr-brt force-pushed the 4177-data-plane-selector-control-api branch from 1044d45 to 1833335 Compare May 16, 2024 15:04
@ndr-brt ndr-brt requested a review from paullatzelsperger May 17, 2024 06:52
@ndr-brt ndr-brt merged commit 0ca80ac into eclipse-edc:main May 17, 2024
16 checks passed
@ndr-brt ndr-brt deleted the 4177-data-plane-selector-control-api branch May 17, 2024 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Feature related to the (REST) api refactoring Cleaning up code and dependencies
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move data plane selection endpoints from management to control api
3 participants