ManageIQ / manageiq-automation_engine

Automation engine for ManageIQ
Apache License 2.0
11 stars 74 forks source link

Replace deprecated with_clean_env with unbundled_env #421

Closed d-m-u closed 4 years ago

d-m-u commented 4 years ago

Bundler v2.1.4 has this: [DEPRECATED] 'Bundler.with_clean_env' has been deprecated in favor of 'Bundler.with_unbundled_env'.

See https://github.com/rubygems/bundler/pull/6843

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 3676


Files with Coverage Reduction New Missed Lines %
lib/miq_automation_engine/engine/miq_ae_engine/drb_remote_invoker.rb 1 87.23%
lib/miq_automation_engine/engine/miq_ae_method_service/miq_ae_service_front.rb 1 87.5%
lib/miq_automation_engine/engine/miq_ae_method_service/miq_ae_service_object_common.rb 1 78.38%
lib/miq_automation_engine/service_models/miq_ae_service_service_reconfigure_request.rb 1 75.0%
lib/miq_automation_engine/service_models/mixins/miq_ae_service_miq_provision_mixin.rb 1 75.0%
lib/miq_automation_engine/engine/miq_ae_engine/miq_ae_object.rb 2 90.13%
lib/miq_automation_engine/service_models/miq_ae_service_miq_provision_request.rb 2 79.31%
lib/miq_automation_engine/service_models/miq_ae_service_miq_request_task.rb 2 87.5%
lib/miq_automation_engine/service_models/miq_ae_service_miq_request.rb 3 84.0%
lib/miq_automation_engine/service_models/miq_ae_service_vm.rb 3 73.08%
<!-- Total: 357 -->
Totals Coverage Status
Change from base Build 3663: -5.7%
Covered Lines: 4723
Relevant Lines: 5923

💛 - Coveralls
coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 3925


Files with Coverage Reduction New Missed Lines %
lib/miq_automation_engine/engine/miq_ae_engine/drb_remote_invoker.rb 1 85.11%
lib/miq_automation_engine/engine/miq_ae_engine/miq_ae_object.rb 1 88.79%
lib/miq_automation_engine/engine/miq_ae_method_service/miq_ae_service_front.rb 1 87.5%
lib/miq_automation_engine/engine/miq_ae_method_service/miq_ae_service_object_common.rb 1 78.38%
lib/miq_automation_engine/service_models/miq_ae_service_service_reconfigure_request.rb 1 75.0%
lib/miq_automation_engine/service_models/miq_ae_service_miq_provision_request.rb 2 79.31%
lib/miq_automation_engine/service_models/miq_ae_service_miq_request_task.rb 2 87.5%
lib/miq_automation_engine/service_models/mixins/miq_ae_service_miq_provision_mixin.rb 2 75.0%
lib/miq_automation_engine/service_models/miq_ae_service_miq_request.rb 3 84.0%
lib/miq_automation_engine/service_models/miq_ae_service_vm.rb 3 73.08%
<!-- Total: 350 -->
Totals Coverage Status
Change from base Build 3923: -5.9%
Covered Lines: 4754
Relevant Lines: 5942

💛 - Coveralls
d-m-u commented 4 years ago

Just kidding.

gmcculloug commented 4 years ago

@d-m-u This one has been in the back of my mind for a while and I think I played with the same change at one point. I assume from the you closing and the previous comment that it is not a simple replace. What results did you get?

d-m-u commented 4 years ago

@gmcculloug yeah, everything broke and, just as you said, not a trivial change. Will make you chat with me about it next week.

d-m-u commented 4 years ago

https://travis-ci.com/github/ManageIQ/manageiq-cross_repo-tests/builds/155884456 is my proof that this isn't going to be as problematic as we were originally thinking

jrafanie commented 4 years ago

copying what we chatted about... this looks great... with_unbundled_env is the same as with_clean_env so I think this is good to do.

The original testing we did for with_clean_env involved testing requiring different gems like this:

Compiled gem in bundle: irb(main):001:0> require 'nokogiri'; puts Nokogiri::VERSION 1.10.9

Pure ruby gem in bundle from gemset: irb(main):002:0> require 'elif'; puts Elif::VERSION 0.1.0

Git based gem in bundle from gemset: irb(main):003:0> require 'manageiq-gems-pending'; puts ManageIQ::Gems::Pending::VERSION 0.1.0

New gem installed on the system: irb(main):004:0> require 'timecop'; puts Timecop::VERSION 0.9.1

Updated version of a gem already in the bundle: irb(main):005:0> require 'faraday'; puts Faraday::VERSION 0.9.2

If those all work in an automate ruby method, I think we can merge this.

miq-bot commented 4 years ago

Checked commit https://github.com/d-m-u/manageiq-automation_engine/commit/edee77e199affee1832efd778d971d1d870d4778 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint 1 file checked, 0 offenses detected Everything looks fine. :trophy:

d-m-u commented 4 years ago

this is the version list for before my awful bundler update on an upstream appliance and after:

@jrafanie looks fine can we merge please? :trollface:

before: require 'nokogiri'; puts Nokogiri::VERSION => 1.8.5 require 'faraday'; puts Faraday::VERSION => 0.9.2 require 'elif'; puts Elif::VERSION => 0.1.0 require 'linux_admin'; puts LinuxAdmin::VERSION => 2.0.0

after: XXX nokogiri version: 1.8.5 XXX faraday version: 1.0.1 XXX elif version: 0.1.0 XXX linux_admin version: 2.0.0

jrafanie commented 4 years ago

Depends on https://github.com/ManageIQ/manageiq/pull/20015

jrafanie commented 4 years ago

LGTM. @simaishi What do you think of upgrading bundler? This PR and https://github.com/ManageIQ/manageiq/pull/20015 would need to go back?

simaishi commented 4 years ago

Sounds good to me!

gmcculloug commented 4 years ago

Yes @kbrock, but the question is how far far away is the galaxy.

d-m-u commented 4 years ago

not to be that person, @gmcculloug, but you're missing a comma

gmcculloug commented 4 years ago

Found it!