to use proxy for rollbar monitoring #119
Conversation
lib/pushmi_pullyu/cli.rb
Outdated
| host: options[:rollbar_proxy_host], | ||
| port: options[:rollbar_proxy_port] | ||
| } | ||
|
|
There was a problem hiding this comment.
Layout/EmptyLinesAroundBlockBody: Extra empty line detected at block body end. (https://github.com/bbatsov/ruby-style-guide#empty-lines-around-bodies)
examples/pushmi_pullyu.yml
Outdated
| @@ -17,6 +17,8 @@ process_name: pushmi_pullyu | |||
| queue_name: dev:pmpy_queue | |||
| minimum_age: 0 | |||
| rollbar_token: 'abc123xyz' | |||
| rollbar_proxy_host: 'your_proxy_host_url' | |||
There was a problem hiding this comment.
Wondering if it be better to name space rollbar now? like the rest?
rollbar:
token: 'etc'
proxy_host: 'etc'
proxy_url: 'etc
Also might not hurt to write a test to make sure these are being set accordingly? (copy and paste the rollbar_token test if you want)
lib/pushmi_pullyu/cli.rb
Outdated
| @@ -58,6 +58,10 @@ def configure_rollbar | |||
| Rollbar.configure do |config| | |||
| config.enabled = false unless options[:rollbar_token].present? | |||
| config.access_token = options[:rollbar_token] | |||
| config.proxy = { | |||
| host: options[:rollbar_proxy_host], | |||
There was a problem hiding this comment.
assuming rollbar still works if these are nils and just doesnt use proxy?
There was a problem hiding this comment.
the spec test still passes, without the two values set up in fixtures/config.yml, so it should be working without the proxy values
There was a problem hiding this comment.
Well the spec test literally just checks if the config file gets read and initializes the option hash. Doesn't actually test if rollbar gets configured or working i don't think. Haven't read into rollbar. I'm assuming it works (if it does this is fine)? Or maybe it doesn't (then we need to do a present check or something) ? ¯\(ツ)/¯
| @@ -225,7 +229,7 @@ def shutdown | |||
| exit!(1) | |||
| else | |||
| # using stderr instead of logger as it uses an underlying mutex which is not allowed inside trap contexts. | |||
| $stderr.puts 'Exiting... Interrupt again to force quit.' | |||
| warn 'Exiting... Interrupt again to force quit.' | |||
There was a problem hiding this comment.
theres a test that makes this fail. Requiring updating it. Look at travisCI build
There was a problem hiding this comment.
Take a peak at changing the test to using these maybe?
https://relishapp.com/rspec/rspec-expectations/docs/built-in-matchers/output-matcher
Doc has examples with warn
|
Okay - will do.
One thing comes up in fixing a rubocop error here:
https://github.com/ualbertalib/pushmi_pullyu/pull/119/files/9fb8a729f0a4e1c42ef9960f8e845a6c63e708b3#diff-8d1e664a20e3c2b30fe4f01a34477acdL228
This breaks a spec test: /spec/pushmi_pullyu/cli_spec.rb:71
Not sure how to fix it. still reading on it.
*Weiwei Shi*
Digital Initiative Applications Librarian
University of Alberta Libraries
2-10L Cameron Library
Edmonton, Alberta, Canada T6G 2J8
Phone:(780)492-7802
Fax: (780)248-1209
Email: weiwei.shi@ualberta.ca
…On Fri, Nov 3, 2017 at 4:10 PM, Shane Murnaghan ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In examples/pushmi_pullyu.yml
<#119 (comment)>
:
> @@ -17,6 +17,8 @@ process_name: pushmi_pullyu
queue_name: dev:pmpy_queue
minimum_age: 0
rollbar_token: 'abc123xyz'
+rollbar_proxy_host: 'your_proxy_host_url'
Wondering if it be better to name space rollbar now? like the rest?
token: 'etc'
proxy_host: 'etc'
proxy_url: 'etc
Also might not hurt to write a test to make sure these are being set
accordingly? (copy and paste the rollbar_token test if you want)
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#119 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AB8-fo9uxSXcXExRxGjAAF8_k9fz7AF6ks5sy49EgaJpZM4QRuMH>
.
|
|
thanks @murny |
To configure rollbar to use proxy.