-
Notifications
You must be signed in to change notification settings - Fork 16
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
Ensuring rails_options are parsed #102
Open
jeremyf
wants to merge
1
commit into
cbeer:master
Choose a base branch
from
jeremyf:updating-rails-option-parsing
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains 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
Prior to this commit, I had an `.engine_cart.yml` file with the following YAML/ERB: ```yaml <% rails_options = [] rails_options << "--skip-listen" if ENV.fetch('RAILS_VERSION', '') < '5.0' rails_options << "--database=postgresql" if ENV.fetch('CI', false) %> rails_options: "<%= rails_options.join(' ') %>" ``` The rendered YAML was: `rails_options: "--skip-listen --database=postgresql"` When I ran the `rake engine_cart:generate` the parameters passed to `Rails::Generators::AppGenerator.start` were; ```ruby [ "internal", "--skip-git", "--skip-keeps", "--skip_spring", "--skip-bootsnap", "--skip-listen", "--skip-bundle", "--skip-test --database=postgresql" ] ``` Note, the last parameter, wihch cam from the `.engine_cart.yaml` are not split. The end result is that the Rails generation did not build the `.internal_test_app` with postgresql as the given database option. In introducing this change, the above parameters were: ```ruby [ "internal", "--skip-git", "--skip-keeps", "--skip_spring", "--skip-bootsnap", "--skip-listen", "--skip-bundle", "--skip-test", "--database=postgresql" ] ``` With those parameters, the `.internal_test_app` was built with postgresql as the default Rails database. Also, to help any debugging, I'm adding an output to the generation process to debug what configuration options I'm doing. In this debugging, I disocvered that the `.engine_cart.yml` config completely obliterates `ENGINE_CART_RAILS_OPTIONS` ENV. That is acceptable but means some downstream implementations may not be configuring their engine cart behavior as they thought, in particular looking at [Hyrax's CircleCI Config][1] as it interplays with it's [`.engine_cart.yml` configuration][2] [1]:https://github.com/samvera/hyrax/blob/977e2d0719ba0d5163fc8719f86d0a094a8fb27e/.circleci/config.yml#L67 [2]:https://github.com/samvera/hyrax/blob/977e2d0719ba0d5163fc8719f86d0a094a8fb27e/.engine_cart.yml
jeremyf
added a commit
to samvera/hyrax
that referenced
this pull request
Feb 4, 2020
At present, EngineCart's parsing of `.engine_cart.yml` will overwrite the `ENGINE_CART_RAILS_OPTIONS` set in CircleCI (see [cbeer/engine_cart/102][1] for some details). The present overwrite behavior is reasonable. As the removed `.engine_cart.yml` targeted pre-Rails 5 behavior, I opted to remove it. In doing so, this would now "activate" the `.circleci/config.yml` declaration of `ENGINE_CART_RAILS_OPTIONS`. As we have coffee script in Hyrax (for better or worse) and we have ActionCable usage, I needed to revisit the now activated parameters. Note: This is in preparation for allowing PostgreSQL as the database for CircleCI. As this impacts the `.internal_test_app` generation, I also needed to bump the Regen number. TL;DR: A 4 year old file that we no longer need is silently trampling on a CI variable. That CI variable has likely not been used. It now will be used, but needed modifications to reflect that usage. _I hope this works, as iterating on the CI process is always time consuming as it requires coordination of remote resources._ [1]:cbeer/engine_cart#102
jeremyf
added a commit
to samvera/hyrax
that referenced
this pull request
Feb 4, 2020
At present, EngineCart's parsing of `.engine_cart.yml` will overwrite the `ENGINE_CART_RAILS_OPTIONS` set in CircleCI (see [cbeer/engine_cart/102][1] for some details). The present overwrite behavior is reasonable. As the removed `.engine_cart.yml` targeted pre-Rails 5 behavior, I opted to remove it. In doing so, this would now "activate" the `.circleci/config.yml` declaration of `ENGINE_CART_RAILS_OPTIONS`. As we have coffee script in Hyrax (for better or worse) and we have ActionCable usage, I needed to revisit the now activated parameters. Note: This is in preparation for allowing PostgreSQL as the database for CircleCI. As this impacts the `.internal_test_app` generation, I also needed to bump the Regen number. TL;DR: A 4 year old file that we no longer need is silently trampling on a CI variable. That CI variable has likely not been used. It now will be used, but needed modifications to reflect that usage. _I hope this works, as iterating on the CI process is always time consuming as it requires coordination of remote resources._ [1]:cbeer/engine_cart#102
jeremyf
added a commit
to samvera/hyrax
that referenced
this pull request
Feb 4, 2020
At present, EngineCart's parsing of `.engine_cart.yml` will overwrite the `ENGINE_CART_RAILS_OPTIONS` set in CircleCI (see [cbeer/engine_cart/102][1] for some details). The present overwrite behavior is reasonable. As the removed `.engine_cart.yml` targeted pre-Rails 5 behavior, I opted to remove it. In doing so, this would now "activate" the `.circleci/config.yml` declaration of `ENGINE_CART_RAILS_OPTIONS`. As we have coffee script in Hyrax (for better or worse) and we have ActionCable usage, I needed to revisit the now activated parameters. Note: This is in preparation for allowing PostgreSQL as the database for CircleCI. As this impacts the `.internal_test_app` generation, I also needed to bump the Regen number. TL;DR: A 4 year old file that we no longer need is silently trampling on a CI variable. That CI variable has likely not been used. It now will be used, but needed modifications to reflect that usage. _I hope this works, as iterating on the CI process is always time consuming as it requires coordination of remote resources._ [1]:cbeer/engine_cart#102
jeremyf
added a commit
to samvera/hyrax
that referenced
this pull request
Feb 4, 2020
At present, EngineCart's parsing of `.engine_cart.yml` will overwrite the `ENGINE_CART_RAILS_OPTIONS` set in CircleCI (see [cbeer/engine_cart/102][1] for some details). The present overwrite behavior is reasonable. As the removed `.engine_cart.yml` targeted pre-Rails 5 behavior, I opted to remove it. In doing so, this would now "activate" the `.circleci/config.yml` declaration of `ENGINE_CART_RAILS_OPTIONS`. As we have coffee script in Hyrax (for better or worse) and we have ActionCable usage, I needed to revisit the now activated parameters. Note: This is in preparation for allowing PostgreSQL as the database for CircleCI. As this impacts the `.internal_test_app` generation, I also needed to bump the Regen number. TL;DR: A 4 year old file that we no longer need is silently trampling on a CI variable. That CI variable has likely not been used. It now will be used, but needed modifications to reflect that usage. _I hope this works, as iterating on the CI process is always time consuming as it requires coordination of remote resources._ [1]:cbeer/engine_cart#102
jeremyf
added a commit
to samvera/hyrax
that referenced
this pull request
Feb 4, 2020
At present, EngineCart's parsing of `.engine_cart.yml` will overwrite the `ENGINE_CART_RAILS_OPTIONS` set in CircleCI (see [cbeer/engine_cart/102][1] for some details). The present overwrite behavior is reasonable. As the removed `.engine_cart.yml` targeted pre-Rails 5 behavior, I opted to remove it. In doing so, this would now "activate" the `.circleci/config.yml` declaration of `ENGINE_CART_RAILS_OPTIONS`. As we have coffee script in Hyrax (for better or worse) and we have ActionCable usage, I needed to revisit the now activated parameters. Note: This is in preparation for allowing PostgreSQL as the database for CircleCI. As this impacts the `.internal_test_app` generation, I also needed to bump the Regen number. TL;DR: A 4 year old file that we no longer need is silently trampling on a CI variable. That CI variable has likely not been used. It now will be used, but needed modifications to reflect that usage. _I hope this works, as iterating on the CI process is always time consuming as it requires coordination of remote resources._ [1]:cbeer/engine_cart#102
jeremyf
added a commit
to samvera/hyrax
that referenced
this pull request
Feb 4, 2020
At present, EngineCart's parsing of `.engine_cart.yml` will overwrite the `ENGINE_CART_RAILS_OPTIONS` set in CircleCI (see [cbeer/engine_cart/102][1] for some details). The present overwrite behavior is reasonable. As the removed `.engine_cart.yml` targeted pre-Rails 5 behavior, I opted to remove it. In doing so, this would now "activate" the `.circleci/config.yml` declaration of `ENGINE_CART_RAILS_OPTIONS`. As we have coffee script in Hyrax (for better or worse) and we have ActionCable usage, I needed to revisit the now activated parameters. Note: This is in preparation for allowing PostgreSQL as the database for CircleCI. As this impacts the `.internal_test_app` generation, I also needed to bump the Regen number. TL;DR: A 4 year old file that we no longer need is silently trampling on a CI variable. That CI variable has likely not been used. It now will be used, but needed modifications to reflect that usage. _I hope this works, as iterating on the CI process is always time consuming as it requires coordination of remote resources._ [1]:cbeer/engine_cart#102
jeremyf
added a commit
to samvera/hyrax
that referenced
this pull request
Feb 4, 2020
At present, EngineCart's parsing of `.engine_cart.yml` will overwrite the `ENGINE_CART_RAILS_OPTIONS` set in CircleCI (see [cbeer/engine_cart/102][1] for some details). The present overwrite behavior is reasonable. As the removed `.engine_cart.yml` targeted pre-Rails 5 behavior, I opted to remove it. In doing so, this would now "activate" the `.circleci/config.yml` declaration of `ENGINE_CART_RAILS_OPTIONS`. As we have coffee script in Hyrax (for better or worse) and we have ActionCable usage, I needed to revisit the now activated parameters. The parameters to which I changed are the defaults of EngineCart (as of this PR). Note: This is in preparation for allowing PostgreSQL as the database for CircleCI. As this impacts the `.internal_test_app` generation, I also needed to bump the Regen number. TL;DR: A 4 year old file that we no longer need is silently trampling on a CI variable. That CI variable has likely not been used. It now will be used, but needed modifications to reflect that usage. _I hope this works, as iterating on the CI process is always time consuming as it requires coordination of remote resources._ [1]:cbeer/engine_cart#102
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.
Prior to this commit, I had an
.engine_cart.yml
file with thefollowing YAML/ERB:
The rendered YAML was:
rails_options: "--skip-listen --database=postgresql"
When I ran the
rake engine_cart:generate
the parameters passed toRails::Generators::AppGenerator.start
were;Note, the last parameter, wihch cam from the
.engine_cart.yaml
arenot split. The end result is that the Rails generation did not build
the
.internal_test_app
with postgresql as the given database option.In introducing this change, the above parameters were:
With those parameters, the
.internal_test_app
was built withpostgresql as the default Rails database.
Also, to help any debugging, I'm adding an output to the generation
process to debug what configuration options I'm doing.
In this debugging, I disocvered that the
.engine_cart.yml
configcompletely obliterates
ENGINE_CART_RAILS_OPTIONS
ENV. That isacceptable but means some downstream implementations may not be
configuring their engine cart behavior as they thought, in particular
looking at Hyrax's CircleCI Config as it interplays with it's
.engine_cart.yml
configuration