Skip to content

Add proper Ractor support to URI - #26

Merged
hsbt merged 3 commits into
ruby:masterfrom
eregon:fix-ractor-support
Jun 25, 2021
Merged

hsbt merged 3 commits into
ruby:masterfrom
eregon:fix-ractor-support

Conversation

@eregon

@eregon eregon commented Jun 25, 2021

Copy link
Copy Markdown
Member

This avoids the drawbacks/hacks of both #22 and #15.

It uses a module to map scheme names to scheme classes, which also works with Ractor.

eregon added 2 commits June 25, 2021 14:18
* This reverts commit 1faa4fd.
* It has too many problems, see ruby#22 for discussion.
* Using a module to map scheme name to scheme class, which also works with Ractor.
* No constant redefinition, no ObjectSpace, still fast lookup for initial schemes.
This was referenced Jun 25, 2021
@hsbt

hsbt commented Jun 25, 2021

Copy link
Copy Markdown
Member

Can you pick #25 with this pull-request?

@eregon

eregon commented Jun 25, 2021

Copy link
Copy Markdown
Member Author

@hsbt Done!

@eregon

eregon commented Jun 25, 2021

Copy link
Copy Markdown
Member Author

@hsbt OK to merge this? CI is green and it fixes the bug found in #25 + the other advantages.

@hsbt

hsbt commented Jun 25, 2021

Copy link
Copy Markdown
Member

👍 Thanks!

@hsbt
hsbt merged commit cdfb3f5 into ruby:master Jun 25, 2021
byroot added a commit to rails/rails that referenced this pull request Jul 27, 2021
ruby/uri#26 was recently pulled
in ruby-head, and it breaks globalid < 0.5.
@byroot

byroot commented Jul 27, 2021 •

Copy link
Copy Markdown
Member

This seem to break support of schemes that aren't valid ruby constants, e.g:

Error:
ActiveRecord::ConnectionAdapters::MergeAndResolveDefaultUrlConfigTest#test_url_with_hyphenated_scheme:
NameError: wrong constant name IBM-DB
 
    if !uri_class && !const_name.empty? && Schemes.const_defined?(const_name, false)
                                                  ^^^^^^^^^^^^^^^
    /usr/local/lib/ruby/3.1.0/uri/common.rb:94:in `const_defined?'
    /usr/local/lib/ruby/3.1.0/uri/common.rb:94:in `for'
    /usr/local/lib/ruby/3.1.0/uri/rfc2396_parser.rb:210:in `parse'
    /rails/activerecord/lib/active_record/database_configurations/connection_url_resolver.rb:25:in `initialize'
    /rails/activerecord/lib/active_record/database_configurations/url_config.rb:48:in `new'
    /rails/activerecord/lib/active_record/database_configurations/url_config.rb:48:in `build_url_hash'
    /rails/activerecord/lib/active_record/database_configurations/url_config.rb:38:in `initialize'
    /rails/activerecord/lib/active_record/database_configurations.rb:262:in `new'
    /rails/activerecord/lib/active_record/database_configurations.rb:262:in `environment_url_config'
    /rails/activerecord/lib/active_record/database_configurations.rb:253:in `block in merge_db_environment_variables'
    /rails/activerecord/lib/active_record/database_configurations.rb:250:in `map'
    /rails/activerecord/lib/active_record/database_configurations.rb:250:in `merge_db_environment_variables'
    /rails/activerecord/lib/active_record/database_configurations.rb:180:in `build_configs'
    /rails/activerecord/lib/active_record/database_configurations.rb:19:in `initialize'
    /rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:26:in `new'
    /rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:26:in `resolve_db_config'
    /rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:157:in `test_url_with_hyphenated_scheme'
 
rails test rails/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb:154

https://buildkite.com/rails/rails/builds/79689#6a1b7cfc-fa17-4752-bc27-62158a228e79/1018-1029

@byroot

byroot commented Jul 27, 2021

Copy link
Copy Markdown
Member

-, . and + are symbols that can be found in valid registered schemes: https://www.iana.org/assignments/uri-schemes/uri-schemes.xhtml

smortex added a commit to riemann/riemann-tools that referenced this pull request Feb 13, 2023
URI.register_scheme was added in ruby/uri#26 and
is part of `uri` 0.11.0.

URI 0.11.0 was imported in Ruby in
ruby/ruby@8ef125c
and is part of Ruby 3.1.0+.
smortex added a commit to riemann/riemann-tools that referenced this pull request Feb 13, 2023
URI.register_scheme was added in ruby/uri#26 and
is part of `uri` 0.11.0.

URI 0.11.0 was imported in Ruby in
ruby/ruby@8ef125c
and is part of Ruby 3.1.0+.

The CI did not catch it because `3.0` in YAML == `3` and `Ruby 3` is
`Ruby 3.2`.
maxlazio pushed a commit to gitlabhq/omnibus-gitlab that referenced this pull request May 15, 2023
ruby/uri#26 refactored how schemes are
registered in the `uri` gem. Ruby 3.1.4 - 3.2.2 ships with uri
v0.12.1, which has this change.

Changelog: fixed
rtokarek-fastly added a commit to rtokarek-fastly/net-scp that referenced this pull request Jun 21, 2024
Ruby 3.1 adopted uri 0.11.0. That version changes how schemes can be registered. There is now a register_scheme method.

ruby/uri#26 as part of https://github.com/ruby/uri/releases/tag/v0.11.0

This commit supports both ways and works with the new method if available. The old way errors out when run  on uri>=0.11.0/ruby>=3.1.
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.

3 participants