Conversation
…o Script manager;
|
@forgems Mr Świderski - this PR corrects all bugs that you reported - please take a look and share your thoughts. The sooner the better! ;) |
syncano/models/manager.py
Outdated
| @clone | ||
| def get_runtimes(self): | ||
| """ | ||
| Method which get available runtimes from CORE; |
syncano/models/manager.py
Outdated
| return self.model._meta.resolve_endpoint(self.endpoint, defaults), defaults | ||
|
|
||
|
|
||
| class RuntimeChoices(object): |
There was a problem hiding this comment.
IMHO it's not needed. You just removed the validator from runtime_name, so why bother with this ?
Also why not simply return a list ? When you set attributes for an object you have to do dir(object) instead just print a list.
There was a problem hiding this comment.
The list was in first version. Thought that would be nicer in such 'container'; the usage:
RUNTIMES = Script.please.get_runtimes()
# and later
Script.please.create(runtime_name=RUNTIMES.PHP, ...)
# rather than:
RUNTIMES = Script.please.get_runtimes()
# and later
Script.please.create(runtime_name=RUNTIMES[2], ...)
There was a problem hiding this comment.
As I understood, you wanted to implement something that reasembles metaenum, but your solution doesn't work like this, because you set attributes on class instance, not on a class itself.
There was a problem hiding this comment.
So why simply not "Script.runtime_name.choices" then? You could define a field with @cached_property choices, and make a request from there.
There was a problem hiding this comment.
Is it a big deal here? :)
That this is an class instance and not class itself?
There was a problem hiding this comment.
Yep - thinkin bout that :) That user must do some inspection.
How do you want to provide constants, if runtime_names can change? dynamically?
Do you have any idea?
There was a problem hiding this comment.
And as for solution with list - user must do some inspection too! :)
There was a problem hiding this comment.
print(choices) vs print(dir(choices)) is shorter ;)
There was a problem hiding this comment.
Few more comments and I will remove this get_runtimes functionality ;)
|
lgtm |
No description provided.