Multiple Updates #14
Multiple Updates #14
Conversation
| @@ -60,7 +50,15 @@ class BaseClient(object): | |||
|
|
|||
| def __init__(self, username, | |||
| password=None, | |||
| url=VALIDATION_FRAMEWORK_URL): | |||
There was a problem hiding this comment.
I don't like this change. We need to be able to support different URLs and different client ids, for example when running a local development server, beyond the two cases here.
I suggest adding client_id as a keyword argument to the constructor, with a default value of the production server, and then have an optional config.json file in the project directory where you can read alternative values for the url and client id (could also be used for other project-level configuration, like a regex or glob expression for which output files get uploaded to storage).
| @@ -180,6 +194,59 @@ def _hbp_auth(self, username, password): | |||
| else: | |||
| raise Exception("Something went wrong. Status code {} from NMPI, expected 302".format(rNMPI1.status_code)) | |||
|
|
|||
| def _translate_URL_to_UUID(self, path): | |||
There was a problem hiding this comment.
This method should be in the CollabDataStore class.
There was a problem hiding this comment.
Will be done in #17
| else: | ||
| raise Exception("The provided 'path' is invalid! Error: " + str(data)) | ||
|
|
||
| def _download_resource(self, uuid): |
There was a problem hiding this comment.
This method should be in the CollabDataStore class.
There was a problem hiding this comment.
Will be done in #17
| @@ -212,9 +279,10 @@ class TestLibrary(BaseClient): | |||
| password : string, optional | |||
| Your HBP Collaboratory password; advisable to not enter as plaintext. | |||
| If left empty, you would be prompted for password at run time (safer). | |||
| url : string, optional | |||
There was a problem hiding this comment.
Same comment as above. We need to keep url field.
| @@ -20,7 +21,8 @@ | |||
| from datetime import datetime | |||
| from . import TestLibrary, ModelCatalog | |||
| from .datastores import CollabDataStore | |||
|
|
|||
| from fpdf import FPDF | |||
There was a problem hiding this comment.
I suggest making these dependencies optional, i.e. generate_report() produces an error message saying "Please install these packages ...." if they are not installed.
There was a problem hiding this comment.
Will be done in #18
Two Fixes: Boolean as String changed; Valid values dict removed from API calls
Multiple updates such as: