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

Ensure that Figaro.adapter is set to Figaro::Rails::Application before the figaro Railtie is loaded #260

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

hlascelles
Copy link

Figaro should be able to be used in the initializer of a Rails engine. Unfortunately, when testing the engine using a tool like Combustion, we see that the Railtie to load figaro is added immediately after the definition of Figaro::Rails::Application.

This can (and sometimes does) mean that the before_configuration block of that Railtie is immediately executed. At this point, the Figaro.adapter is still nil, so it is lazily created as a Figaro::Application. The Railtie then calls Figaro.load on it, and it blows up as default_path throws a NotImplementedError.

The solution is to set the adapter to a Figaro::Rails::Application as soon as possible, and before the Railtie is created.

This PR makes that 2 line swap change, and adds Combustion to the specs. If the two lines of logic are not swapped, you can see how the initializing process fails.

@joehorsnell
Copy link

joehorsnell commented Feb 16, 2017

@hlascelles - your builds are failing because the Travis config picks up different Bundler gemfiles for different Ruby versions (in figaro/gemfiles) so you'll need to add combustion to each of those.

@hlascelles
Copy link
Author

hlascelles commented Nov 30, 2022

This change has been extracted and added (hlascelles/figjam#7) to a fork and continuation of this repo, and a new gem called Figjam: https://github.com/hlascelles/figjam

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.

None yet

2 participants