Return url for file in GCP public bucket instead of error #748
Conversation
Deploying datachain-documentation with
|
| Latest commit: |
d2e2e06
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e72679fe.datachain-documentation.pages.dev |
| Branch Preview URL: | https://gcp-get-public-bucket-file-u.datachain-documentation.pages.dev |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #748 +/- ##
==========================================
- Coverage 87.47% 87.46% -0.01%
==========================================
Files 114 114
Lines 10941 10949 +8
Branches 1504 1505 +1
==========================================
+ Hits 9571 9577 +6
- Misses 992 993 +1
- Partials 378 379 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| return self.fs.sign(self.get_full_path(path), expiration=expires, **kwargs) | ||
| except AttributeError as exc: | ||
| is_anon = self.fs.storage_options.get("token") == "anon" | ||
| if is_anon and "you need a private key to sign credentials" in str(exc): |
There was a problem hiding this comment.
a few questions come to my mind: :)
can we return a public link right away if anon is set to True?
is there any guarantee that HTTP link this always work for public buckets?
do we do anything like this for S3? if not, why?
when does this error happen?
There was a problem hiding this comment.
a few questions come to my mind: :)
These are good questions! 👍
can we return a public link right away if anon is set to True?
Yes, we can. I doubted about chosen option here and still not sure, but on the other side I just wanted to be sure I will not break any existing use case.
is there any guarantee that HTTP link this always work for public buckets?
Yes: https://cloud.google.com/storage/docs/access-public-data#api-link
do we do anything like this for S3? if not, why?
We definitely should do the same for other types of storages. I am not doing it here for some reasons:
- I wanted to check first if this will solve all the issues in SaaS
- it will requires more work and proper tests, I am not sure we have capacity now for this (fix for GCP was quick and simple)
when does this error happen?
Original issue is here: https://github.com/iterative/studio/issues/11118
There was a problem hiding this comment.
Original issue is here: https://github.com/iterative/studio/issues/11118
yep, I understand. I guess my question is why it is happening. If anon it passed AFAIU it should not be even trying to use credentials (I assume it is failing on a cluster because some default Google cred are present, right?)
There was a problem hiding this comment.
yep, I understand. I guess my question is why it is happening. If anon it passed AFAIU it should not be even trying to use credentials (I assume it is failing on a cluster because some default Google cred are present, right?)
Ah, got it. It is failing not in a workers cluster, but on the backend (API request). It is interesting, I can not find any default Google credentials on backend 🤔
When retrieving URL for file in GCP bucket if this bucket is public and we have no GCP credentials set, error will be raised.
In this PR we are returning public download URL in this case instead of exception.
Note: I think we need to do the same for other types of storages.