Skip to content

Resolve default implementation issue for list_keys in Provide #312

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

Closed
ionut-arm opened this issue Jan 8, 2021 · 2 comments · Fixed by #321
Closed

Resolve default implementation issue for list_keys in Provide #312

ionut-arm opened this issue Jan 8, 2021 · 2 comments · Fixed by #321
Assignees
Labels
bug Something isn't working

Comments

@ionut-arm
Copy link
Member

For most of the methods on the Provide trait we offer a default implementation that generally just returns PsaErrorNotSupported. This allows developers of providers to skip the implementation of those methods in particular if they're not supported for their backend or if they haven't done the implementation work yet.

However, providing these defaults doesn't always work as expected - the list_keys operation, for example, is service-wide so one provider not supporting it would currently break the operation for the whole service.

There are two possible solutions:

  • remove the default implementation for list_keys
  • keep the implementation, but allow the core provider to simply skip when it sees PsaErrorNotSupported as the returned error

Neither of those really fixes the problem - developers should really implement that functionality at some point and there's no way for us to know if they've done that currently. So I also propose updating the all_providers::normal::list_keys test to loop through all providers returned by list_providers and to generate a key for each one that supports generate_key (and maybe try import if generate isn't supported?). Then we can check that they were all created.

@ionut-arm ionut-arm added the bug Something isn't working label Jan 8, 2021
@ionut-arm ionut-arm self-assigned this Jan 8, 2021
@ionut-arm
Copy link
Member Author

Note that this should also be relevant to other operations that work on a service-aggregate level, such as those in #311

@ionut-arm
Copy link
Member Author

Also implement similar changes for list_clients and delete_client

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant