Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List profile versions from blob storage #575

Merged
merged 2 commits into from
Jan 14, 2022
Merged

List profile versions from blob storage #575

merged 2 commits into from
Jan 14, 2022

Conversation

jenhagg
Copy link
Collaborator

@jenhagg jenhagg commented Jan 14, 2022

Pull Request doc

Purpose

Currently the way we do this involves parsing a version.json file located in blob storage which must match the versions that are actually available. However to list locally available profiles, we check the files directly, which is more reliable than using the json file. This PR makes it so we use the same code for both local and blob storage, with the only difference being which filesystem is provided.

What the code is doing

See above. It is a refactor to be more consistent and eliminate the need for the version.json file.

Testing

Ran the _local and _cloud functions interactively. Added new test for the shared logic (which was already in use, but untested).

Time estimate

10 min

@jenhagg jenhagg self-assigned this Jan 14, 2022
@jenhagg jenhagg added the refactor Code that is being refactored label Jan 14, 2022
Comment on lines +20 to +40
def get_profile_version_cloud(grid_model, kind):
"""Returns available raw profile from blob storage.

:param str grid_model: grid model.
:param str kind: *'demand'*, *'hydro'*, *'solar'* or *'wind'*.
:return: (*list*) -- available profile version.
"""
bfs = fs.open_fs("azblob://besciences@profiles").opendir(f"raw/{grid_model}")
return _get_profile_version(bfs, kind)


def get_profile_version_local(grid_model, kind):
"""Returns available raw profile from local file.

:param str grid_model: grid model.
:param str kind: *'demand'*, *'hydro'*, *'solar'* or *'wind'*.
:return: (*list*) -- available profile version.
"""
profile_dir = fs.path.join(server_setup.LOCAL_DIR, "raw", grid_model)
lfs = fs.open_fs(profile_dir)
return _get_profile_version(lfs, kind)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether there is an easy way to combine these two functions together and make location of the profiles a parameter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be a simplification we could do, say if we had the local and blob filesystems already created as part of some DataAccess object. This seemed like the simplest approach for now though.

Copy link
Collaborator

@BainanXia BainanXia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@jenhagg jenhagg merged commit 80d21da into develop Jan 14, 2022
@jenhagg jenhagg deleted the jon/get_version branch January 14, 2022 22:13
@rouille
Copy link
Collaborator

rouille commented Jan 15, 2022

Very nice. I have never been a fan of the json file

@jenhagg
Copy link
Collaborator Author

jenhagg commented Jan 18, 2022

Very nice. I have never been a fan of the json file

Same. I've been looking forward to this PR ever since adding the json file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code that is being refactored
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants