refactor: modify artifact-id logic; jsonify responses #390
Conversation
jupyterjazz
requested review from
bwanglzu,
gmastrapas,
guenthermi,
jemmyshin and
violenil
|
Tests aren't passing here because we haven't updated |
finetuner/client/client.py
Outdated
| artifact_ids = self.get_run(experiment_name=experiment_name, run_name=run_name)[ | ||
| ARTIFACT_IDS | ||
| ] |
There was a problem hiding this comment.
this part let me clarify it to you, i think it is wrong.
finetuner/constants.py
Outdated
| @@ -19,3 +19,4 @@ | |||
| EVAL_DATA = 'eval_data' | |||
| FINETUNED_MODELS_DIR = 'finetuned_models' | |||
| MODEL = 'model' | |||
| ARTIFACT_IDS = 'FINETUNER_ARTIFACT_IDS' | |||
There was a problem hiding this comment.
as mentioned above, it is the env variable we send to AWS batch, not the field we saved in the database.
There was a problem hiding this comment.
but how do I, as a client, retrieve it then?
| @@ -47,16 +47,16 @@ def _handle_request( | |||
| method: str, | |||
| params: Optional[dict] = None, | |||
| json_data: Optional[dict] = None, | |||
| ) -> requests.Response: | |||
| ) -> Union[dict, List[dict]]: | |||
There was a problem hiding this comment.
nevermind, I guess we want to support >=3.7
finetuner/client/client.py
Outdated
|
|
||
| :param experiment_name: The name of the experiment. | ||
| :param run_name: The name of the run. | ||
| :param path: Directory where the model will be stored. | ||
| :returns: A str object indicates the download path on localhost. | ||
| :returns: A list of str object(s) that indicate the download path on localhost. |
There was a problem hiding this comment.
localhost? You mean local disk?
There was a problem hiding this comment.
I honestly just copied that from hubble's download_artifact docstring. But I see your point, it's a bit confusing here.
|
test is failing |
Tests are failing here because we haven't updated |
Closes https://github.com/jina-ai/finetuner.fit/issues/209 and https://github.com/jina-ai/finetuner.fit/issues/210
requests.Responseobjects