Allow poll interval to be specified from the command line. #52
Conversation
|
|
||
| describe Coursemology::Evaluator::CLI::Options do | ||
| it { is_expected.to have_attributes(host: nil, api_token: nil, api_user_email: nil) } | ||
| it { is_expected.to have_attributes(host: nil, api_token: nil, api_user_email: nil, poll_time: nil, poll_interval: nil) } |
| describe Coursemology::Evaluator::CLI::Options do | ||
| it { is_expected.to have_attributes(host: nil, api_token: nil, api_user_email: nil) } | ||
| it { is_expected.to have_attributes(host: nil, api_token: nil, api_user_email: nil, | ||
| poll_time: nil, poll_interval: nil) } |
There was a problem hiding this comment.
Expression at 20, 77 should be on its own line.
| describe Coursemology::Evaluator::CLI::Options do | ||
| it { is_expected.to have_attributes(host: nil, api_token: nil, api_user_email: nil) } | ||
| it 'checks Options attributes' do | ||
| expect(subject).to have_attributes(host: nil, api_token:nil, api_user_email: nil, |
fonglh
force-pushed
the
custom-poll-time
branch
2 times, most recently
from
81f4995 to
1b340a2
Compare
lib/coursemology/evaluator/client.rb
Outdated
| # This sleep might not be triggered in the specs, because interruptions to the thread is | ||
| # nondeterministically run by the OS scheduler. | ||
| sleep(1.minute) | ||
| sleep(@poll_time.send(@poll_interval)) |
There was a problem hiding this comment.
This is very susceptible to injection.
There was a problem hiding this comment.
Also, I'm not sure if poll intervals are being tested
There was a problem hiding this comment.
No they aren't, I tested this manually.
There was a problem hiding this comment.
You can stub out sleep, and check that the correct timeout is specified as a spec
|
@allenwq suggested just forcing the units to be in seconds. That'll remove the need for parsing and removes the dependency on the ISO8601 gem. The number can also be passed directly to the sleep function. |
Add tests for checking the parsing of the poll interval option. Parse the interval string with the ISO8601 gem and use the value in the client loop. Specify default values of optional parameters in a consistent way.
|
But since you have done it already, it's OK to stick with the current way. Just that I feel using seconds is enough. |
|
I just remembered I also want to do customizable Docker image cache interval. That could vary over a much greater time frame so I'll still need the ISO8601 gem. |
|
|
||
| it 'sleeps with the default poll time' do | ||
| # Simulate no evaluations | ||
| expect(subject).to receive(:allocate_evaluations) { [] } |
fonglh
force-pushed
the
custom-poll-time
branch
2 times, most recently
from
6972d62 to
44c7df2
Compare
| expect(subject).to receive(:sleep).with(10) | ||
| subject.run | ||
| end | ||
|
|
There was a problem hiding this comment.
Extra empty line detected at block body end.
Check that the correct poll times are being used.
|
LGTM |
|
I will merge this first since @fonglh need it for deploy. |
Uses 10 seconds as the default.
Fixes #46.