-
Notifications
You must be signed in to change notification settings - Fork 362
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
Update keyword arguments for Ruby 3.0 #2651
Conversation
Co-authored-by: Seth Boyles <sboyles@pivotal.io> Co-authored-by: Mona Mohebbi <mmohebbi@pivotal.io>
@@ -43,7 +43,7 @@ def cp_r_to_blobstore(*args) | |||
error_handling { wrapped_client.cp_r_to_blobstore(*args) } | |||
end | |||
|
|||
def download_from_blobstore(*args) | |||
def download_from_blobstore(*args, **kwargs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get where these kwargs
are coming from...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They are used here:
cloud_controller_ng/lib/cloud_controller/blobstore/fog/fog_client.rb
Lines 41 to 49 in 5ee5e06
def download_from_blobstore(source_key, destination_path, mode: nil) | |
FileUtils.mkdir_p(File.dirname(destination_path)) | |
File.open(destination_path, 'wb') do |file| | |
(@cdn || files).get(partitioned_key(source_key)) do |*chunk| | |
file.write(chunk[0]) | |
end | |
file.chmod(mode) if mode | |
end | |
end |
Due to the multi-layer wrapping of these classes, each method needs to conform to the same signature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See here for where they get wrapped:
cloud_controller_ng/lib/cloud_controller/blobstore/client_provider.rb
Lines 28 to 56 in 4a23a41
def provide_fog(options, directory_key, root_dir) | |
cdn_uri = HashUtils.dig(options[:cdn], :uri) | |
cdn = CloudController::Blobstore::Cdn.make(cdn_uri) | |
client = FogClient.new( | |
connection_config: options.fetch(:fog_connection), | |
directory_key: directory_key, | |
cdn: cdn, | |
root_dir: root_dir, | |
min_size: options[:minimum_size], | |
max_size: options[:maximum_size], | |
storage_options: options[:fog_aws_storage_options] | |
) | |
logger = Steno.logger('cc.blobstore') | |
# work around https://github.com/fog/fog/issues/3137 | |
# and Fog raising an EOFError SocketError intermittently | |
# and https://github.com/fog/fog-aws/issues/264 | |
# and https://github.com/fog/fog-aws/issues/265 | |
# and intermittent GCS blobstore download errors | |
errors = [Excon::Errors::BadRequest, Excon::Errors::SocketError, SystemCallError, | |
Excon::Errors::InternalServerError, Excon::Errors::ServiceUnavailable, | |
Google::Apis::ServerError, Google::Apis::TransmissionError | |
] | |
retryable_client = RetryableClient.new(client: client, errors: errors, logger: logger) | |
Client.new(ErrorHandlingClient.new(SafeDeleteClient.new(retryable_client, root_dir))) | |
end |
…words"" This reverts commit 536ab5b. * we have been unable to reproduce the failures on other environments. Reverting the revert to see if it was a pipeline issue
…y/ruby-3-keywords"" This reverts commit 536ab5b. * we have been unable to reproduce the failures on other environments. Reverting the revert to see if it was a pipeline issue
Pulled this out of #2623 to avoid having long living branches and merge conflicts with them.
See here for why this needs to be changed: https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/