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

Implement FileStatus.uri property #1648

Merged
merged 10 commits into from
Aug 5, 2020
Merged

Implement FileStatus.uri property #1648

merged 10 commits into from
Aug 5, 2020

Conversation

asvetlov
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Jul 29, 2020

Codecov Report

Merging #1648 into master will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1648      +/-   ##
==========================================
+ Coverage   76.48%   76.50%   +0.02%     
==========================================
  Files          57       57              
  Lines        8657     8666       +9     
  Branches     1426     1427       +1     
==========================================
+ Hits         6621     6630       +9     
  Misses       1836     1836              
  Partials      200      200              
Flag Coverage Δ
#unit 76.50% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
neuromation/api/storage.py 92.00% <100.00%> (+0.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8688128...a502e5f. Read the comment docs.

Copy link
Contributor

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Update docs/storage_reference.rst.

CHANGELOG.D/1648.feature Outdated Show resolved Hide resolved
base_uri: Optional[URL], values: Dict[str, Any]
) -> FileStatus:
if base_uri is None:
uri = URL("storage://" + values["path"].lstrip("/"))
Copy link
Contributor

Choose a reason for hiding this comment

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

What if the path contains characters %, ? or #?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean %?# in cluster or user name?
Is it allowed at all?

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently they are not allowed in cluster name and user name. But user can create files with arbitrary names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got you.
@serhiy-storchaka please review the updated version

Copy link
Contributor

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

👍

@asvetlov
Copy link
Contributor Author

asvetlov commented Aug 5, 2020

Thanks @serhiy-storchaka for the very careful review!

@asvetlov asvetlov merged commit c3c2314 into master Aug 5, 2020
@asvetlov asvetlov deleted the storage-uri branch August 5, 2020 20:53
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.

2 participants