flysystem-google-cloud-storage
flysystem-google-cloud-storage copied to clipboard
Better object ACL support
Allow the default bucket ACL to be applied when visibility is not explicitly specified, and copy the actual ACL when copying an object.
Still need to update the tests...
@matthewgoslett All tests here pass except for the PHP nightly. That failure is actually due to changes in the count()
function and the fact that the older version of Mockery is performing a count(null)
when we use the withNoArgs()
check.
We have a couple of options here:
- don't use the
withNoArgs()
check. - Move the minimum supported version of the project to 5.6 (5.5 went EOL almost a year ago) and update the versions phpunit and Mockery used in the project.
- Ignore the nightly failure.
Also, @matthewgoslett, I made the default when visibility
is not explicit to not set an ACL (predefined or otherwise) so that the bucket's default ACL would be applied as expected. I realize that this could potentially be a breaking change and am open to making this behavior dependent on an extra boolean parameter to the constructor (that defaults to the existing behavior) if you think that's prudent, though I would argue that this behavior is more correct.
Reference: https://cloud.google.com/storage/docs/json_api/v1/defaultObjectAccessControls
Is there any expectation from flysystem as to what the default visibility should be?
@matthewgoslett Not in the base adapters. If you look at the local and FTP adapters, for instance, the visibility is only set if visibility
is defined in the write
, etc calls or via a default in the filesystem config. Otherwise, Flysystem won't force a given permission, letting the default filesystem umask/etc take effect.
Same is actually true for the S3 adapter.
@mgriego apologies for allowing this to become stale. Could you merge the latest master into this branch? We no longer test against PHP:nightly
so the tests should pass then.
This solves and issue for me, anything I can do to help get the PR moved along?
@nicja are you able to merge this PR?
I've just had to effectively monkey patch my own system to get what I needed. https://www.kublermdk.com/2022/01/29/googlecloud-flysystem-tweaks-to-support-uniform-bucket-level-access/
I assume others are doing the same given this PR has been sitting around for years.
@kublermdk unfortunately I am no longer an admin on this repo.
@matthewgoslett are you able to merge this PR? It's been sitting here for a long time. Alternatively can you add others as admins so they can maintain the codebase? Or do you know who has access?
Hey
I'm not admin of this repo and haven't been involved for many years. You'll need to ping someone from the https://github.com/Superbalist organisation. I really don't know anyone who'd be able to assist here.
On Mon, 31 Jan 2022 at 09:29, Michael Kubler @.***> wrote:
@matthewgoslett https://github.com/matthewgoslett are you able to merge this PR? It's been sitting here for a long time. Alternatively can you add others as admins so they can maintain the codebase? Or do you know who has access?
— Reply to this email directly, view it on GitHub https://github.com/Superbalist/flysystem-google-cloud-storage/pull/57#issuecomment-1025451673, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAL7XH57GGK2U6LBWPEDR5DUYY25XANCNFSM4DJ62VTA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
You are receiving this because you were mentioned.Message ID: @.*** com>