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

Refactor Github actions CI to correctly load gemfile #817

Merged
merged 5 commits into from
Dec 14, 2020

Conversation

kmcphillips
Copy link
Contributor

@kmcphillips kmcphillips commented Dec 10, 2020

Problem

The exported env does not persist between steps, so the export BUNDLE_GEMFILE does nothing.

You can see that it loads the current 5.1 for every build step:
Add_note_about_keeping_Shopify_shopify_up_to_date___816__·_Shopify_shopify_api_6c93dc0

Solution

I've refactored the config to use jobs.build.env config instead.

I've cleaned up and refactored a few things too, but that's the major change.

We could use bundler-cache: true but that may not work with the bundler config settings, so I've left it out.

Now we have a new problem

The build appears to have been broken for a while. There are also a ton of deprecation warnings.

I'm going to look into it but will probably need help from owners.

@kmcphillips kmcphillips requested a review from a team as a code owner December 10, 2020 16:43
@andyw8
Copy link
Contributor

andyw8 commented Dec 10, 2020

Thanks @kmcphillips.

fyi @mllemango

@andyw8
Copy link
Contributor

andyw8 commented Dec 10, 2020

We just recently switched from Travis to GitHub actions in https://github.com/Shopify/shopify_api/pull/808/files so I don't expect much would have broken in that short time.

@andyw8
Copy link
Contributor

andyw8 commented Dec 10, 2020

(but worth nothing that we previously had allow_failures for when running against AR master).

@mllemango
Copy link
Contributor

I believe the new deprecation messages are coming from activesupport v6.1.0, we used to use v.6.0.3.1 without any problems. Not sure how to approach this other than specifying gem "activesupport", "6.0.3.1" in the Gemfile.

@andyw8
Copy link
Contributor

andyw8 commented Dec 10, 2020

We can ignore the deprecation messages for the moment, let's fix the time-related issues:

NoMethodError: undefined method `hours' for 24:Integer

It seems to fail inconsistently so may be related to load order and the test randomization.

@andyw8
Copy link
Contributor

andyw8 commented Dec 10, 2020

Looks like we need to add require 'active_support/time' to session.rb.

@andyw8
Copy link
Contributor

andyw8 commented Dec 10, 2020

(the intermittent failures may be because GitHub is caching the gem downloads prior to the Rails 6.1 release. But I can't see any way in the UI to to force that to clear.).

@andyw8
Copy link
Contributor

andyw8 commented Dec 11, 2020

Still not fully sure of the cause of this yet, perhaps there was a transitive dependency on ActiveSupport. But let's make that explicit anyway.

@andyw8 andyw8 changed the title Refactor Github actions CI to correctly load gemfile Refactor Github actions CI to correctly load gemfile (and support Rails 6.1.0) Dec 11, 2020
@@ -1,6 +1,7 @@
# frozen_string_literal: true
require 'openssl'
require 'rack'
require 'active_support/time'
Copy link
Contributor

Choose a reason for hiding this comment

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

If #818 is merged then we don't need this.

@andyw8 andyw8 changed the title Refactor Github actions CI to correctly load gemfile (and support Rails 6.1.0) Refactor Github actions CI to correctly load gemfile Dec 14, 2020
@andyw8 andyw8 force-pushed the gh-actions-gemfile branch from fe04a0d to be7452b Compare December 14, 2020 15:12
@andyw8 andyw8 merged commit 0e434d2 into master Dec 14, 2020
@andyw8 andyw8 deleted the gh-actions-gemfile branch December 14, 2020 15:40
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