Feature/issue 6 submission function #7
Conversation
There was a problem hiding this comment.
Looks good so far. I have the following suggestions:
- To make this even simpler, you could remove the
__init__constructor and makesubmit_reporta static method that accepts a url, series id, series token, and report - make
urla keyword argument, with a default value of the test endpoint, e.g.
def submit_report(..., url="http://localhost:4500/reports")
There's also no test code for this module. When you add your unit tests, assume that there is a running testbed api at http://localhost:4500, and that it has the two test report series we discussed
There was a problem hiding this comment.
In the last review, I mentioned:
There's also no test code for this module. When you add your unit tests, assume that there is a running testbed api at http://localhost:4500, and that it has the two test report series we discussed
I'm still not seeing any test code, which means there's no way to know if the function works correctly or breaks in the future. Can you please add tests?
|
Whoops I missed that part. Adding the tests in today. |
| ''' | ||
| header = {"GA4GH-TestbedReportSeriesId": series_id, "GA4GH-TestbedReportSeriesToken": series_token} | ||
| submit_request = requests.post(url, headers=header ,json=report) | ||
| return submit_request.status_code |
There was a problem hiding this comment.
Returning the status code from this method is insufficient, especially if there's been an error we want to know why (i.e. the message). If the submission has been successful, then we want the id of the new report that was created on the server. So,
- Always return status code
- Return error message if submission wasn't successful,
Noneif submission was successful - Return new report id if submission was successful,
Noneif submission wasn't successful
Should be pretty easy to do in a Python dictionary
Created a submission function for making post requests to the testbed api