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

download datum://id and datum://alias files, run execution locally, using project data files #134

Merged
merged 2 commits into from
Jun 25, 2024

Conversation

ccschneidr
Copy link
Contributor

@ccschneidr ccschneidr commented Jun 21, 2024

As discussed in the Continental support channel, we need the possibility to download datum:// input files to run an execution locally. This pull request adds this feature to valohai-utils' download.py.
Using this we were able to run a complete execution, locally.

To test, perform the following steps

  • log in to valohai with vh login
  • link a project, if you want to use aliases with vh link
  • execute python snippet (make sure to replace my_models_alias and 01941935-832f-16d4-af69-6a9bf6bf4df6 with existent data)
import valohai

valohai.prepare(
    step="predict",
    default_inputs={
        "model-http": "http://example.com",
        "model-alias": "datum://my_models_alias",
        "model-id": "datum://01941935-832f-16d4-af69-6a9bf6bf4df6",
    },
)

print(valohai.inputs("model-http").path())
print(valohai.inputs("model-id").path())
print(valohai.inputs("model-alias").path())

@hylje
Copy link
Contributor

hylje commented Jun 24, 2024

Thank you for the PR, I will adopt the branch to add improvements and add automated test cases.

@hylje hylje force-pushed the master branch 2 times, most recently from 4383332 to 1f96f9a Compare June 25, 2024 07:14
@hylje hylje requested a review from memona008 June 25, 2024 07:15
Copy link
Contributor

@memona008 memona008 left a comment

Choose a reason for hiding this comment

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

LGTM! ❇️ some nits though

tests/test_download.py Outdated Show resolved Hide resolved
tests/test_download.py Outdated Show resolved Hide resolved
- Add automated tests for datum URL downloading
- Revert changes to argument names; although these are internal APIs,
  they may be used in the wild and we can avoid breaking existing usage
@memona008 memona008 merged commit 4f2e23c into valohai:master Jun 25, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants