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

(MODULES-11126) Replacing URI.escape with URI::DEFAULT_PARSER #1195

Merged
merged 1 commit into from
Sep 20, 2021

Conversation

valleedelisle
Copy link
Contributor

@valleedelisle valleedelisle commented Jul 22, 2021

URI.escape was deprecated in ruby-2.7 and removed in ruby-3.x. This
change is breaking stdlib. This is described in detail here [1].

Replacing URI.escape with URI::DEFAULT_PARSER is working properly.

This is affecting openstack-tripleo [2]

[1] https://bugs.ruby-lang.org/issues/17309
[2] https://bugs.launchpad.net/tripleo/+bug/1937166

@valleedelisle valleedelisle requested a review from a team as a code owner July 22, 2021 04:47
@CLAassistant
Copy link

CLAassistant commented Jul 22, 2021

CLA assistant check
All committers have signed the CLA.

@puppet-community-rangefinder
Copy link

uriescape is a function

Breaking changes to this file MAY impact these 4 modules (near match):

This module is declared in 319 of 577 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@karelyatin
Copy link

There is also an open issue for this https://tickets.puppetlabs.com/browse/MODULES-11126, so can also link that.

@valleedelisle valleedelisle changed the title Replacing URI.escape with URI::DEFAULT_PARSER (MODULES-11126) Replacing URI.escape with URI::DEFAULT_PARSER Jul 22, 2021
b4ldr
b4ldr previously approved these changes Jul 27, 2021
@b4ldr b4ldr dismissed their stale review July 27, 2021 11:56

failing tests

@b4ldr
Copy link
Collaborator

b4ldr commented Jul 27, 2021

@valleedelisle thanks for the PR, are you able to look at the failing spec tests, thanks

URI.escape was deprecated in ruby-2.7 and removed in ruby-3.x. This
change is breaking stdlib.

Replacing URI.escape with URI::DEFAULT_PARSER is working properly.
@valleedelisle
Copy link
Contributor Author

@valleedelisle thanks for the PR, are you able to look at the failing spec tests, thanks

I believe I've fixed it based on my local test, I apologize for the lost cycles.

@valleedelisle
Copy link
Contributor Author

To me, the CI failure we see in the check is not related to this patch:

  localhost: {"_error"=>{"kind"=>"provision/docker_failure", "msg"=>"Attempted to run\ncommand:'docker exec litmusimage_oraclelinux_6-2222 yum install -y sudo openssh-server openssh-clients'\nstdout:Loaded plugins: ovl, security, ulninfo\nSetting up Install Process\n\nstderr:rpmdb: unable to join the environment\nerror: db3 error(11) from dbenv->open: Resource temporarily unavailable\nerror: cannot open Packages index using db3 - Resource temporarily unavailable (11)\nerror: cannot open Packages database in /var/lib/rpm\nTraceback (most recent call last):\n  File \"/usr/bin/yum\", line 29, in <module>\n    yummain.user_main(sys.argv[1:], exit_code=True)\n  File \"/usr/share/yum-cli/yummain.py\", line 298, in user_main\n    errcode = main(args)\n  File \"/usr/share/yum-cli/yummain.py\", line 146, in main\n    result, resultmsgs = base.doCommands()\n  File \"/usr/share/yum-cli/cli.py\", line 440, in doCommands\n    return self.yum_cli_commands[self.basecmd].doCommand(self, self.basecmd, self.extcmds)\n  File \"/usr/share/yum-cli/yumcommands.py\", line 211, in doCommand\n    return base.installPkgs(extcmds)\n  File \"/usr/share/yum-cli/cli.py\", line 702, in installPkgs\n    self.install(pattern=arg)\n  File \"/usr/lib/python2.6/site-packages/yum/__init__.py\", line 3666, in install\n    if (self.rpmdb.searchNames([po.name]) and\n  File \"/usr/lib/python2.6/site-packages/yum/rpmsack.py\", line 1198, in searchNames\n    returnList.extend(self._search(name=name))\n  File \"/usr/lib/python2.6/site-packages/yum/rpmsack.py\", line 1277, in _search\n    mi = ts.dbMatch('name', name)\n_rpm.error: rpmdb open failed\n", "backtrace"=>["/tmp/60cffbc7-ba6d-444f-8399-721cf60520d1/provision/lib/task_helper.rb:21:in `run_local_command'", "/tmp/60cffbc7-ba6d-444f-8399-721cf60520d1/provision/tasks/docker.rb:31:in `install_ssh_components'", "/tmp/60cffbc7-ba6d-444f-8399-721cf60520d1/provision/tasks/docker.rb:157:in `provision'", "/tmp/60cffbc7-ba6d-444f-8399-721cf60520d1/provision/tasks/docker.rb:198:in `<main>'"], "details"=>{}}}}

@valleedelisle
Copy link
Contributor Author

@b4ldr

Any other feedback?

Thanks,

DVD

@b4ldr
Copy link
Collaborator

b4ldr commented Aug 23, 2021

@valleedelisle sorry been on vacation for the last three weeks, i agree i don't think this is related. i have kicked of the ci again hoping it was a transient issue will recheck this tomorrow

@b4ldr
Copy link
Collaborator

b4ldr commented Aug 23, 2021

@david22swan i think this is related to cento6 EOL i.e. unrelated to the PR. i can't override can you? or should we just drop cento6 and rebase

@puppet-community-rangefinder
Copy link

uriescape is a function

that may have no external impact to Forge modules.

This module is declared in 318 of 578 indexed public Puppetfiles.


These results were generated with Rangefinder, a tool that helps predict the downstream impact of breaking changes to elements used in Puppet modules. You can run this on the command line to get a full report.

Exact matches are those that we can positively identify via namespace and the declaring modules' metadata. Non-namespaced items, such as Puppet 3.x functions, will always be reported as near matches only.

@carabasdaniel
Copy link
Contributor

Hi @b4ldr @valleedelisle, closed and re-opened the PR to re-trigger the tests. The cla might need to be signed to be able to merge this, but overall it looks good. Thanks for the contribution.

Copy link
Member

@david22swan david22swan left a comment

Choose a reason for hiding this comment

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

LGTM

@david22swan david22swan merged commit 7929a21 into puppetlabs:main Sep 20, 2021
traylenator added a commit to traylenator/puppetlabs-stdlib that referenced this pull request Oct 17, 2023
This reverts commit 799d608.

While uriescape was deprecated for Puppet 8 in puppetlabs#1307
it was already fixed earlier for Puppet 8 and ruby 3 in puppetlabs#1195

It is unclear to me why this function was deprecated.

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

Successfully merging this pull request may close these issues.

6 participants