-
Notifications
You must be signed in to change notification settings - Fork 6
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
Quote Engine Non-CDN Retrieval #48
base: master
Are you sure you want to change the base?
Conversation
600c701
to
1a159c7
Compare
@wazaundtechnik since you said using raw Github is enough for our usage, is this PR still relevant? |
This PR allows hot reload every arbitrary interval (default to 5
minutes). Previously, reload only happens on program startup. So yes,
this PR is still relevant.
|
@wazaundtechnik wow that's cool. okay then. can @fushar take a look at this PR too? now that we have another user (beside our own group) maybe it's time to work on this again hehe. |
|
||
|
||
@pytest.fixture | ||
def qe(): |
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.
can you please use a more descriptive name? it's not obvious what qe
stands for.
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.
before that, why did you put this as fixture? as I understand it, this is your system under test. shouldn't you explicitly instantiate QuoteEngine
inside your test case and not provide this as fixture? this is not going to be used anywhere else (at least for now).
Too tired today... still WIP. I will update this later.
Update: Done everything. Finally!