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

MinterState isn't requirable by implementors. #29

Closed
tpendragon opened this issue Jul 1, 2016 · 13 comments
Closed

MinterState isn't requirable by implementors. #29

tpendragon opened this issue Jul 1, 2016 · 13 comments

Comments

@tpendragon
Copy link
Contributor

Makes production deploys fail.

@tpendragon tpendragon changed the title MinterState isn't requirable by outside libraries. MinterState isn't requirable by implementors. Jul 1, 2016
@atz
Copy link
Contributor

atz commented Jul 1, 2016

Is requirable in engine_cart's .internal_test_app. Probably related to whether the consumer has run rails generate active_fedora:noid:install (as the test app does), is a rails app/engine, etc.

Anything else you can say about your production context that might contribute? Or how to reproduce deterministically in a test? Maybe a separate run of non-engine_cart specs without loading the test app? Then a full pass inside the test app?

Note: 2.x version is currently a beta release.

@atz
Copy link
Contributor

atz commented Jul 7, 2016

@mjgiarlo This is an entanglement of having the ActiveRecord and non-AR code in the same gem. The consumer of the Noid PORO know what they want and don't expect to have to do any generation/installation. The consumers of the AR MinterState presumably also know what they want, but can be expected to do generation/installation. Probably there is some magic we can use to autoload only under certain conditions (defined? ActiveRecord doesn't seem good enough, since a rails app may just want the PORO). So I'm not sure how much is wise to employ.

More particularly, we can separate the test suite components, but I'm unsure there is any clean way to invoke, say, Part A without engine_cart, then everything inclusive of Part A (again) with engine_cart, in the same test run. I can do it with an ENV in travis and 2 separate runs, but don't see how to get it in one.

@mjgiarlo
Copy link
Member

mjgiarlo commented Jul 7, 2016

In the meantime, perhaps we'd best pin our stack to AF::Noid < 2 while we work this out. No, not that.

I wonder if we need to add instructions about how to update an app to work with AF::Noid 2 in the CC 1.1.0 release notes.

@mjgiarlo
Copy link
Member

mjgiarlo commented Jul 7, 2016

Or, alternatively, if that change should have necessitated a CC 2.0.0 release since it's breaking folks.

@atz
Copy link
Contributor

atz commented Jul 7, 2016

That change is in an AF::Noid beta. Nobody should be using it in production, now or ever.

Certainly I think some docs (here) about running the installer would be useful, at least to the AR consumer. But the main question is whether a PORO will be able to still use the same gem without any additional requirements (and how to test that is true).

@tpendragon
Copy link
Contributor Author

tpendragon commented Jul 18, 2016

The generator doesn't run (complains about expand_path?) and copying the migrations over manually and running it doesn't seem to let it run in prod. :(

Edit: Sorry I'm not being terribly helpful here, I can't seem to put together a good way to make this deterministic. I wonder how much is from building this as an engine after the fact, versus using the engine generator

@tpendragon
Copy link
Contributor Author

@atz Does someone have a production app with beta AF::Noid? Because even in CC's internal_app I can't get a MinterState object.

@atz
Copy link
Contributor

atz commented Jul 25, 2016

Again: nobody should be using a beta in production. I'm unaware of anyone who is. That being said, I would appreciate more detail about the generator not running. That would be a dealbreaker.

I agree that complication might come from retrofitting an engine on an existing gem. My initial instinct was to make a separate gem.

@atz
Copy link
Contributor

atz commented Jul 29, 2016

@mjgiarlo need direction on this. In particular, consider the problem of testing with engine cart and simultaneously without it.

@mjgiarlo
Copy link
Member

mjgiarlo commented Aug 1, 2016

I'll do some testing this week or not long after. Nope, that never happened.

See #35 and #36

@val99erie
Copy link

Regarding @tpendragon 's comment about how to make it deterministic: You can reproduce the problem using the test app that is generated by engine_cart in curation_concerns. (Note that the problem only shows up in production mode)

In the curation_concerns workspace:

  • bundle exec rake engine_cart:generate
  • cd .internal_test_app
  • Edit config/secrets.rb to add a secret_key_base in the production section
  • RAILS_ENV=production bin/rails c

See also:
samvera-deprecated/curation_concerns#1045

@mjgiarlo
Copy link
Member

This is addressed in samvera-deprecated/curation_concerns#1061 (WIP).

@mjgiarlo
Copy link
Member

Problem is in how CC requires AF::Noid. Closing.

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

No branches or pull requests

4 participants