Optional configuration without environment variables #36
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
trusche
commented
Aug 7, 2024
| @@ -20,18 +20,35 @@ def self.configure | |||
| yield(configuration) if block_given? | |||
| end | |||
|
|
|||
| def self.configured? | |||
| self.configuration&.configured? | |||
| end | |||
There was a problem hiding this comment.
I've thrown that in here since we use it in our app to conditionally display passkit-related view code only if the gem is properly configured.
trusche
commented
Aug 7, 2024
| @@ -123,14 +121,18 @@ def generate_json_manifest | |||
| File.write(@manifest_url, manifest.to_json) | |||
| end | |||
|
|
|||
| CERTIFICATE = Rails.root.join(ENV["PASSKIT_PRIVATE_P12_CERTIFICATE"]) | |||
| INTERMEDIATE_CERTIFICATE = Rails.root.join(ENV["PASSKIT_APPLE_INTERMEDIATE_CERTIFICATE"]) | |||
There was a problem hiding this comment.
Constants are evaluated at load time, not runtime, so if those envs are nil, this will raise an error at startup of a rails app with eager_load = true. This bit us in production no less, because in staging we had everything configured correctly, and in local environment classes are not eager loaded, so it didn't manifest there.
trusche
force-pushed
the
main
branch
5 times, most recently
from
e34a641
to
1ecc0c2
Compare
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.
Hi @coorasse,
we just started using your gem in our Rails app, it's working great, so thank you, first of all!
We ran into a bit of trouble deploying this, though. If any of the required env variables is not set, not only will the gem not work (expected and acceptable), but the rails app will fail to start (not so good 😁 ).
This PR adds a couple of configuration changes, and might help addressing both #34 and #5.
true, so no behaviour change on upgrade, it must be set tofalseexplicitly in an initializer)Tests are still passing, but I didn't add any new ones since I'm more familiar with rspec and a bit short on time right now. Happy to amend that sometime later if needed.
Cheers,
Thilo