Support fog-core 2.5 and up #636
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is needed to support fog-core 2.2 and up due to this change: fog/fog-core#241
This is needed to support fog-core 2.2 and up due to this change: fog/fog-core#241
This is needed to support fog-core 2.2 and up due to this change: fog/fog-core#241
This lines up more with Ruby best practices for module and class locations.
|
@Temikus I first ran the storage integration tests: $ bundle exec rake test:storage
/home/stanhu/.local/share/mise/installs/ruby/3.3.7/bin/ruby -I"lib:test" /home/stanhu/.local/share/mise/installs/ruby/3.3.7/lib/ruby/gems/3.3.0/gems/rake-13.2.1/lib/rake/rake_test_loader.rb "test/integration/storage/test_buckets.rb" "test/integration/storage/test_coverage.rb" "test/integration/storage/test_directories.rb" "test/integration/storage/test_files.rb" "test/integration/storage/test_objects.rb"
Started with run options --seed 58113
TestStorageRequests
test_list_object_acl PASS (2.61s)
test_put_bucket PASS (0.86s)
test_files_public_url PASS (1.28s)
test_get_object PASS (1.38s)
test_files_all PASS (2.77s)
test_delete_object PASS (1.50s)
test_put_object_paperclip PASS (1.08s)
test_put_object_invalid_predefined_acl PASS (1.05s)
test_files_set_body_string PASS (1.71s)
test_files_destroy PASS (1.66s)
test_files_get_https_url PASS (0.78s)
test_files_create_file PASS (1.45s)
test_object_metadata PASS (2.69s)
test_directories_destroy PASS (2.76s)
test_copy_object_with_object_property PASS (1.56s)
test_files_copy PASS (1.74s)
test_put_object_string PASS (1.26s)
test_files_create_string PASS (1.57s)
test_put_object_file PASS (1.23s)
test_get_object_https_url PASS (1.32s)
test_put_bucket_invalid_predefined_acl PASS (0.06s)
test_files_each PASS (2.26s)
test_put_object_nil PASS (0.86s)
test_directory_files PASS (1.21s)
test_copy_object PASS (1.70s)
test_files_get PASS (1.11s)
test_files_set_body_file PASS (1.64s)
test_list_objects PASS (2.38s)
test_directories_get PASS (0.90s)
test_list_bucket_acl PASS (1.60s)
test_put_object_acl PASS (1.21s)
test_copy_object_with_request_options PASS (1.50s)
test_put_bucket_acl PASS (1.38s)
test_files_create_predefined_acl PASS (1.18s)
test_directories_all PASS (0.97s)
test_files_get_https_url_whitespace PASS (0.85s)
test_files_metadata PASS (2.00s)
test_get_bucket_acl PASS (1.80s)
test_directory_public_url PASS (1.00s)
test_directories_put_invalid_predefined_acl PASS (0.05s)
test_copy_object_predefined_acl PASS (1.54s)
test_get_bucket PASS (0.81s)
test_directories_put PASS (0.89s)
test_get_object_acl PASS (1.26s)
test_put_object_predefined_acl PASS (1.03s)
test_get_object_http_url PASS (1.10s)
test_put_object_url PASS (0.76s)
test_directories_put_predefined_acl PASS (0.62s)
test_put_object_contradictory_content_type PASS (1.29s)
test_list_buckets PASS (1.03s)
test_delete_bucket PASS (0.92s)
test_files_create_invalid_predefined_acl PASS (1.20s)
test_put_bucket_predefined_acl PASS (0.70s)
Finished in 71.09225s
53 tests, 66 assertions, 0 failures, 0 errors, 0 skipsThen I ran all the integration tests. It looks like the errors are unrelated to these changes: Do you think we can go ahead and merge this? |
This was referenced Mar 19, 2025
stanhu
commented
Mar 19, 2025
| @@ -24,25 +24,25 @@ def class_for(key) | |||
|
|
|||
| def [](service) | |||
| @@connections ||= Hash.new do |hash, key| | |||
| hash[key] = case key | |||
There was a problem hiding this comment.
I think the Ruby LSP on VSCode reformatted this. I don't see an issue here, but I can revert this if you prefer to avoid any unnecessary changes.
|
@icco @plribeiro3000 Would you be able to review this? I'd also like to help maintain this gem: #637. Let me know wat you think. |
Temikus
approved these changes
Apr 25, 2025
| @@ -47,7 +47,7 @@ This project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html | |||
| #### Fixed | |||
|
|
|||
| - #609 Fixed missing paging on all models [agrare] | |||
| - #608 Fixed `Fog::Compute::Google::Servers#all` paging [agrare] | |||
There was a problem hiding this comment.
Nit: I would suggest not to retroactively change the CHANGELOG.md, that might cause some confusion.
| end | ||
| case key | ||
| when :compute | ||
| Fog::Logger.warning("Google[:compute] is not recommended, use Compute[:google] for portability") |
There was a problem hiding this comment.
Nit: Is this still an accurate error with the swap?
| @@ -4,15 +4,15 @@ class << self | |||
| def class_for(key) | |||
| case key | |||
| when :compute | |||
| Fog::Compute::Google | |||
There was a problem hiding this comment.
This is now causing an "uninitialized constant" to be thrown if someone tries to instantiate the old way.
If it's not too much trouble, can you add some shims, e.g.:
module Fog
module Compute
module Google
def self.new(*args)
# Issue a deprecation warning
warn "[DEPRECATION] `Fog::Compute::Google.new` is deprecated. Please use `Fog::Google::Compute.new` instead."
# Redirect the call to the new namespace
Fog::Google::Compute.new(*args)
end
end
end
end
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fog-corev2.2 and up now requiresFog::Google:<service>instead ofFog::<service>::Google: fog/fog-core#241This pull request eliminates these deprecation warnings:
This fixes #421.