-
Notifications
You must be signed in to change notification settings - Fork 20
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
Enable remote draining capability of Drones via REST interface #260
Enable remote draining capability of Drones via REST interface #260
Conversation
Codecov Report
@@ Coverage Diff @@
## master #260 +/- ##
==========================================
+ Coverage 98.81% 98.82% +0.01%
==========================================
Files 54 54
Lines 2192 2215 +23
==========================================
+ Hits 2166 2189 +23
Misses 26 26
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
6873bd0
to
16cc4af
Compare
f8c4b1e
to
91a4be4
Compare
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've only got some minor issues with language/code health. Please see the comments.
tardis/utilities/utils.py
Outdated
@@ -98,6 +99,15 @@ def machine_meta_data_translation( | |||
raise | |||
|
|||
|
|||
def str_to_state(resources): |
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'm somewhat stumped by this function. Looking at the name, I would have assumed it to take a single state name and look that up. Perhaps a name such as load_states
would work better? Do you see benefit in a single-name-lookup str2state
in dronestates
, or is that too trivial?
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.
Yes, load_states
sounds much better. I have renamed it accordingly. I think an additional str2state
function in dronestates
is not reasonable, because the database plugin always returns a listing. In that case with a single element. So, I could either use load_states
and get the first entry in the list or use a str2state
function and put in the first entry from the database listing. So, it is "gehupft wie gesprungen". ;-)
e941638
to
2c1393e
Compare
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.
[storm trooper move along.gif]
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.
👍
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.
[it's a trap approve.gif]
This pull request enables the possibilty to drain drones in
AvailableState
via the REST interface in connection with #250.get_resource_state
method toSqliteRegistry
database
property toDrone
database_state
coroutine toDrone
check_remote_draining
functionAvailableState