Supporting Algo Management API's#18
Supporting Algo Management API's#18besirkurtulmus merged 15 commits intomasteralgorithmiaio/algorithmia-python:masterfrom
Conversation
…r response w/ appropiate error anyway
…nd add the versions() method to algo()
…owercase for .versions() bool query parameters
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.
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().