add sign to filesystem class #277
Conversation
|
Why, what is the license for google.cloud? |
|
I'm not worried about the license for google.cloud, but I thought that you wouldn't want to take on a new dep. There is a version of the code we need written without google.cloud but it is Apache-2 licensed. |
|
Oh, I see. I think we can do without the dep, and reference the origin of the code in the method comments, and mention the implied license. Note that the test is failing on style, please run |
|
@martindurant do you have a test that I should use as a template for this? |
| def sign(self, path, expiration=100, **kwargs): | ||
| # if we don't want to use google.cloud we can use | ||
| bucket, object_name = self.split_path(path) | ||
| self.connect() |
There was a problem hiding this comment.
Why would you need this? Connect should have happened at instantiation.
| request_timestamp = datetime_now.strftime("%Y%m%dT%H%M%SZ") | ||
| datestamp = datetime_now.strftime("%Y%m%d") | ||
|
|
||
| client_email = google_credentials.service_account_email |
There was a problem hiding this comment.
What happens if this isn't a service account?
|
@CJ-Wright : the question about service accounts I think needs to be answered before I know what to do with this PR. The answer "this only works for service accounts" might be OK, but not great - but since I don't really understand how the signing works, I don't know what the other options might be. |
|
There hasnt been any updates on this for a while. I can take over this PR but I dont really understand the whole service account thing. Would it be ok to raise an exception if there is no service account for now? |
|
With #411 I think this can be closed now right? |
This will enable downstream classes to specify how to sign urls, providing short term URLs for situations where the URL is needed but permissioning needs to be tightly controlled.
Note that if we don't want to add a
google.clouddep there is a recipe for how to do this with justgoogle.oauth2which is under an Apache-2.0 License.