feat(api): added import_study method to study_api.py #72
Conversation
There was a problem hiding this comment.
Few remarks:
- The destination_path should be optional if the person just want to import the study. If it's None, we should'nt call the move method
- The
import_studymethod should be renamedimport_study_apiand put at the same place ascreate_study_apias you won't do study.import_study(...), it won't make a lot of sense. This also means the method expects a study_path, and an api_config parameter
tests/integration/test_web_client.py
Outdated
| test_path = Path(antares_web.desktop_path.joinpath("internal_studies").joinpath(study.service.study_id)) | ||
| copy_dir = tmp_path / test_path.name | ||
|
|
||
| tmp_path_zip = tmp_path / f"{copy_dir.name}" |
There was a problem hiding this comment.
You can remove the f-string:
tmp_path_zip = tmp_path / copy_dir.name
tests/integration/test_web_client.py
Outdated
| @@ -92,6 +94,26 @@ def test_creation_lifecycle(self, antares_web: AntaresWebDesktop): | |||
| area_fr.create_wind(ts_matrix) | |||
| assert area_fr.get_wind_matrix().equals(ts_matrix) | |||
|
|
|||
| # testing import study | |||
There was a problem hiding this comment.
Now that this test work we should put everthing you wrote here at the end of the test as it would make the test clearer
tests/integration/test_web_client.py
Outdated
| @@ -92,6 +94,26 @@ def test_creation_lifecycle(self, antares_web: AntaresWebDesktop): | |||
| area_fr.create_wind(ts_matrix) | |||
| assert area_fr.get_wind_matrix().equals(ts_matrix) | |||
|
|
|||
| # testing import study | |||
| test_path = Path(antares_web.desktop_path.joinpath("internal_studies").joinpath(study.service.study_id)) | |||
There was a problem hiding this comment.
Add a little comment saying why we're doing this weird thing
src/antares/craft/model/study.py
Outdated
| wrapper = RequestWrapper(session) | ||
| base_url = f"{api_config.get_host()}/api/v1" | ||
|
|
||
| if has_valid_extension(study_path): |
There was a problem hiding this comment.
I think it will be clearer to first do a
if not has_valid_extension: raise and then continue what you did without the if.
Also the exception you're raising in this case should be a StudyImportError with the custom message you wrote
| base_url = "https://antares.com/api/v1" | ||
| url_import = f"{base_url}/studies/_import" | ||
| with requests_mock.Mocker() as mocker: | ||
| mocker.get(url_import, status_code=404) |
There was a problem hiding this comment.
Don't think you need the mock inside this test as we raise before doing any request I believe
| "folder": None, | ||
| } | ||
|
|
||
| json_ui = { |
There was a problem hiding this comment.
Don't think you need this. you could return an empty json I believe
tests/integration/test_web_client.py
Outdated
| path_test = Path("/new/test/studies") | ||
| imported_study = import_study_api(api_config, zip_study, path_test) | ||
|
|
||
| assert imported_study.path == path_test / f"{imported_study.service.study_id}" |
There was a problem hiding this comment.
We could assert that the imported study has the same areas as the one you copied (to ensure the import went good)
No description provided.