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

Fog::Storage::GoogleJSON::File public/predefined_acl are not passed to api client #499

Closed
wants to merge 2 commits into from

Conversation

swistaczek
Copy link

@swistaczek swistaczek commented Jul 21, 2020

After updating my application to fog-google: 1.10 and ruby 2.7.1 I have noticed that even if my upstream code sets Storage object as public all my files are uploaded as a private one (without any ACL).

I found out that the new Ruby version introduced breaking changes for method arguments.

Change in PR fixes this issue, however, I am not sure if a similar problem is not happening in other places.

CC: @icco, @Temikus and @plribeiro3000

…save (due to changed #put_object signature)
@@ -105,7 +105,7 @@ def save

options[:predefined_acl] ||= @predefined_acl

service.put_object(directory.key, key, body, options)
service.put_object(directory.key, key, body, predefined_acl: options.delete(:predefined_acl), options)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint/Syntax: unexpected token tRPAREN

…save (due to changed #put_object signature)
@Temikus
Copy link
Member

Temikus commented Jul 22, 2020

@swistaczek thanks for getting this to our attention!

So if I'm reading the changes in 2.7 right **options should be sufficient. Have you tried the change just with:

service.put_object(directory.key, key, body, **options)

@swistaczek
Copy link
Author

@Temikus thanks for the reply and for providing a great product to Ruby community! I see your point which is obviously a good catch.

@yosiat
Copy link
Contributor

yosiat commented Oct 1, 2020

Sorry for hijacking this PR, but I found the same issue in copy_object: #505
I can submit another PR to do the same for copy_object if that's acceptable.

@Temikus
Copy link
Member

Temikus commented Dec 2, 2020

@yosiat Yes, totally. Just change the method to **options.
This one should be probably closed in favour of that approach.

@yosiat
Copy link
Contributor

yosiat commented Dec 13, 2020

@Temikus made a PR - #513 let me know if that's ok!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants