Automatically create the directory path for a JSONTokenStorage #998
Conversation
derek-globus
requested review from
aaschaer,
ada-globus,
kurtmckee and
sirosen
as code owners
| ~~~~~ | ||
|
|
||
| - When a JSONTokenStorage is used, the containing directory will be automatically be | ||
| created if it doesn't exist instead of erroring (on Mac). |
There was a problem hiding this comment.
Since it's not mac-specific, I don't think we should note the platform. Since I was suggesting an edit, I also went ahead and restored the pr link text from the template (:pr:`998` also works).
| created if it doesn't exist instead of erroring (on Mac). | |
| created if it doesn't exist instead of erroring. (:pr:`NUMBER`) |
There was a problem hiding this comment.
I'm not sure if it's mac exclusive or not. I only know it doesn't work on mac.
| @@ -40,8 +41,15 @@ def __init__(self, filename: pathlib.Path | str, *, namespace: str = "DEFAULT"): | |||
| :param namespace: A user-supplied namespace for partitioning token data | |||
| """ | |||
| self.filename = str(filename) | |||
| self._ensure_containing_dir_exists() | |||
There was a problem hiding this comment.
If we're adding this, shouldn't we do it on the parent class, not here?
There was a problem hiding this comment.
Sure, that's a fair question. Would you like to open that PR or me?
There was a problem hiding this comment.
Sure, I'll work that up!
(Also, no worries about merge/review order. I noticed it, and we'll sort out now. 😄 )
|
Given that this has already merged, I'm going to open a PR to post-fix the changelog entry and separately ask about moving the ensure_dir behavior up-tree. |
|
Yep, sorry didn't realize you were reviewing & expected this change to be small & uncontroversial enough to not wait around in case someone else was reviewing like I normally try to. |
What?
Why?
~/.globus/appdir didn't exist).📚 Documentation preview 📚: https://globus-sdk-python--998.org.readthedocs.build/en/998/