-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Doppler loader #108
Doppler loader #108
Conversation
lib/anyway/loaders/doppler.rb
Outdated
http = Net::HTTP.new(uri.host, uri.port) | ||
http.use_ssl = true | ||
|
||
# dp.st.dev.9IAsQwBQnFDQsmnyukYTtaJBqjCXJhq8zSFdSvkBVHW |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think its better not to mention sensitive data in the codebase.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
Please, see the comments.
CHANGELOG.md
Outdated
@@ -8,6 +8,10 @@ Steep 1.2+ is required. Read more about the [feature](https://hackmd.io/xLrYaqUt | |||
|
|||
- Added `manifest.yml` for RBS. ([@palkan][]) | |||
|
|||
## 2.3.0 (2023-01-11) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please, do not change the version in the change log; we can not know when this PR will be released
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
lib/anyway/loaders/doppler.rb
Outdated
DOPPLER_REQUEST_ERROR = Class.new(StandardError) | ||
DOPPLER_JSON_FORMAT_URL = "https://api.doppler.com/v3/configs/config/secrets/download" | ||
|
||
def call(**_options) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can probably use forwarding here to not care about the method signature
def call(**_options) | |
def call(...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
but now here I set
(env_prefix:, **_options)
because, I use env_prefix now for Anyway::Env
spec/loaders/doppler_spec.rb
Outdated
allow(http_success).to receive(:read_body) { doppler_content.to_json } | ||
expect_any_instance_of(Net::HTTP).to receive(:request) { http_success } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use webmock
instead; we don't need to care about the underlying HTTP implementation here; it's likely to change over time (e.g., if we decide to add retry mechanism)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
I included it in spec_helper and disabled all net connections
WebMock.disable_net_connect!(allow_localhost: true)
``
…ally doppler loader if DOPPLER_TOKEN exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
What is the purpose of this pull request?
Hello @palkan
Please, review my pr
What changes did you make? (overview)
Add Doppler loader
Checklist
Closes #107