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

Clarification on Storage.Delete #310

Open
anderspitman opened this issue Sep 11, 2024 · 3 comments
Open

Clarification on Storage.Delete #310

anderspitman opened this issue Sep 11, 2024 · 3 comments
Labels
discussion Let's talk about it documentation question Further information is requested

Comments

@anderspitman
Copy link

What is your question?

Under what conditions should Delete return an error? This and this seem to contradict each other.

@anderspitman anderspitman added the question Further information is requested label Sep 11, 2024
@mholt
Copy link
Member

mholt commented Sep 11, 2024

That's a good point. Delete should probably not return fs.ErrNotExist if the key does not exist, but probably should if a prefix of the key does not exist. For example:

  • Delete("a/b/c") when c does not exist inside a/b => No error, since that leaf is effectively deleted.
  • Delete("a/b/c") when a/b does not exist => Probably an error?

I dunno. What do you think, should it return an error in that second case?

@anderspitman
Copy link
Author

I'd be fine with either, though semantically your proposed error essentially boils down to "something I expected to exist doesn't, so inform the caller". So maybe it should return an error in both cases? idk it's not quite the same thing.

In terms of my implementation, what would you guess would be the best behavior to have for now to maximize compatibility with the current release of certmagic?

@mholt
Copy link
Member

mholt commented Sep 13, 2024

I'd have to think on it more. (I'll be busy with a conference presentation for the next week though.)

For now, I'd say just return no error, even if the parent dir(s) does/do not exist, but I want to revisit this thinking at some point.

@mholt mholt added the discussion Let's talk about it label Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Let's talk about it documentation question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants