Supporting Algo Management API's#18
Conversation
…r response w/ appropiate error anyway
…nd add the versions() method to algo()
…owercase for .versions() bool query parameters
zeryx
left a comment
zeryx
left a comment
There was a problem hiding this comment.
Nothing too crazy, however much of the work seems to get performed in the algorithmia_api_client. It would be great to have that linked in here so we can see exactly how stuff gets executed.
| if m is not None: | ||
| self.client = client | ||
| self.path = m.group(1) | ||
| self.username = self.path.split("/")[0] |
There was a problem hiding this comment.
Is it safe to assume here that the algorithm URI structure will always remain the same? Is there a world where this is brittle(supporting unicode algorithm names/usernames?)
There was a problem hiding this comment.
I would assume that if the URI structure ever changes, line 19 would also break (which has been part of the client forever now).
Other than that point, I don't think we'll ever change it.
| settingsObj = Settings(**settings) | ||
| createRequestVersionInfoObj = CreateRequestVersionInfo(**version_info) | ||
| update_parameters = {"details": detailsObj, "settings": settingsObj, "version_info": createRequestVersionInfoObj} | ||
| update_request = UpdateRequest(**update_parameters) |
There was a problem hiding this comment.
Small nit:
Some of your variables are camel case, and some are snake case - I think most of the time you snake case for dicts, but not always. Might be useful to be consistent here.
I think the python default is always snake case?
There was a problem hiding this comment.
Good point. Also as far as I can tell, the client has used both :/
Look at line 19-30. I could change the style for the code I wrote. I personally prefer using underscores. It makes reading the code easier.
There was a problem hiding this comment.
Per python style guide variables should be snake case:
https://www.python.org/dev/peps/pep-0008/#function-and-variable-names
| return api_response | ||
| except ApiException as e: | ||
| error_message = json.loads(e.body)["error"]["message"] | ||
| raise ApiError(error_message) |
There was a problem hiding this comment.
The way we're handling exceptions is throwing away information. What about the error code, etc? Probably doesn't belong in this PR, but something to think about.
4 participants
The new methods are basically wrapper around the generated python client, that can be found here.
I've added the
.create(),.update(),.publish(),.info(),.versions()&.compile()methods underclient.algo().