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

Improve secrets storage async methods error handling #36

Closed
n370 opened this issue Feb 24, 2021 · 6 comments · Fixed by #276
Closed

Improve secrets storage async methods error handling #36

n370 opened this issue Feb 24, 2021 · 6 comments · Fixed by #276
Assignees

Comments

@n370
Copy link
Contributor

n370 commented Feb 24, 2021

No description provided.

@radomird radomird self-assigned this Dec 7, 2021
@radomird
Copy link
Contributor

radomird commented Dec 7, 2021

Hi @n370, did you have anything specific in mind (as the issue lacks description)?

@n370
Copy link
Contributor Author

n370 commented Dec 7, 2021

Hey @radomird, this part of the codebase has largely changed since the last time I worked on it. Maybe @salmanm and @sameer-coder can help you with the specifics of the implementation but my general idea here was to make sure we were properly handling whatever errors happening while saving and/or retrieving secrets from the secure secrets storage mechanism.

@simoneb
Copy link
Member

simoneb commented Dec 7, 2021

Let's see if there's anything we should do in this regard or if the code is already good as is after the recent changes, even though I don't think anything has been done explicitly in this regard.

@radomird
Copy link
Contributor

radomird commented Dec 8, 2021

Thanks @n370 I will go through the code and will add were it is needed.

@salmanm
Copy link
Contributor

salmanm commented Dec 8, 2021

I had refactored storing secrets and simplified the API there. I think it's in a good shape unless there's anything specific that we'd like to implement.

@radomird
Copy link
Contributor

radomird commented Dec 8, 2021

I've added a few unit tests in the PR, the code however looks good as it handles most of the exceptions I've tried. I did find a minor bug which is fixed in the PR as well.

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 a pull request may close this issue.

4 participants