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

Is there a reason to explicitly set the visibility if not passed? #98

Open
phroggyy opened this issue Feb 26, 2019 · 7 comments
Open

Is there a reason to explicitly set the visibility if not passed? #98

phroggyy opened this issue Feb 26, 2019 · 7 comments
Labels
For release To be included in next release

Comments

@phroggyy
Copy link

phroggyy commented Feb 26, 2019

In the following lines, we check if the visibility has been specified, and if it hasn't we default to private. https://github.com/Superbalist/flysystem-google-cloud-storage/blob/master/src/GoogleStorageAdapter.php#L143-L149

Is there any reason we need to default to anything at all? If we specify nothing, the default object ACL of the bucket should be applied (by default private, visible to owner). This seems like more desirable behaviour imo – if users specify a default object ACL on their buckets, I believe it's expected that it will apply to uploaded files.

If there is a good reason this is being done, let's keep it but make it clear in the documentation that we're applying private visibility by default.

@alexking
Copy link

Also note that setting ACLs isn't supported by the new Bucket Only Policy, and causes an error when trying to upload, which makes the driver incompatible with buckets using it.

@nicja
Copy link
Contributor

nicja commented Apr 15, 2019

Could you have a look at PR #57 and see whether it addresses the issue? If so we can clean it up and get it merged

@phroggyy
Copy link
Author

That looks like it would do it indeed @nicja

@nicja
Copy link
Contributor

nicja commented Apr 18, 2019

Thanks, I'm working on getting that merged

@nicja nicja added the For release To be included in next release label Apr 18, 2019
@stepanselyuk
Copy link

Would that be merged anytime soon?

@nicja
Copy link
Contributor

nicja commented Jun 28, 2019

I'm waiting on @mgriego to update #57 so that the tests pass, alternatively if someone else could raise a similar PR that would also help

@kublermdk
Copy link

I've gone down the path of manually updating what I needed to get this done:
https://www.kublermdk.com/2022/01/29/googlecloud-flysystem-tweaks-to-support-uniform-bucket-level-access/

I assume others of you are doing the same given this PR has been sitting around for years.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For release To be included in next release
Projects
None yet
Development

No branches or pull requests

5 participants