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 Edrs controller #215

Merged
merged 1 commit into from
May 2, 2024
Merged

feat: add Edrs controller #215

merged 1 commit into from
May 2, 2024

Conversation

ndr-brt
Copy link
Contributor

@ndr-brt ndr-brt commented Apr 30, 2024

Description

Implement Edrs management endpoint

notes

  • this was tested using prism, that loads a mock server based on the official EDC management API spec as we did on the java client. This will make the tests faster, an issue to translate the other tests to the same method will come, then we could keep just a single end-to-end test.
  • the yaml file used was downloaded from the EDC docs, an issue to download it automatically will come (to handle EDC version updates).

Closes #210

@ndr-brt ndr-brt added the enhancement New feature or request label Apr 30, 2024
@ndr-brt ndr-brt requested a review from fdionisi April 30, 2024 14:04
Copy link
Member

@fdionisi fdionisi left a comment

Choose a reason for hiding this comment

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

Looking great, @ndr-brt! There's really nothing that catches my eyes, but I'm also not super acquainted with the EDR endpoints.

Nice work!

Copy link
Member

Choose a reason for hiding this comment

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

Does it mean we'll be moving away from e2e tests against the connector?

Copy link
Member

Choose a reason for hiding this comment

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

Oh! I see #216 is related to that. I'm a bit hesitant in saying let's go 100% with it; somehow it would be nice to have a e2e test which can ensure everything is wired up correctly, but I admit this is also coming form a place of ignorance. So I would fully trust your direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, in the issue is reported that at least one E2E test should be kept, because otherwise we'll lose coverage, but currently we're testing the same things multiple times because we got a lot of e2e tests that are slow to run (on every new version the connector needs to be built again).
With the new structure every controller test suite will test only its own methods against prism to verify the structure, then few e2e will fill up the uncovered spots

@ndr-brt ndr-brt merged commit 7822f78 into main May 2, 2024
1 check passed
@ndr-brt ndr-brt deleted the 210-edrs-api branch May 2, 2024 06:50
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.

EDR cache api
2 participants