Visitar URL original
Fix handling relative path in normalize_local_path_uri() on Windows. by serhiy-storchaka · Pull Request #815 · neuro-inc/apolo-cli · GitHub
Skip to content

Fix handling relative path in normalize_local_path_uri() on Windows. - #815

Merged
serhiy-storchaka merged 4 commits into
masterfrom
test-windows-absolute-path
May 28, 2019
Merged

Fix handling relative path in normalize_local_path_uri() on Windows.#815
serhiy-storchaka merged 4 commits into
masterfrom
test-windows-absolute-path

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented May 28, 2019

Copy link
Copy Markdown
Contributor

No description provided.

@serhiy-storchaka serhiy-storchaka added the bug Something isn't working label May 28, 2019
@serhiy-storchaka
serhiy-storchaka requested a review from shagren May 28, 2019 09:43
@serhiy-storchaka serhiy-storchaka mentioned this pull request May 28, 2019
Merged
@serhiy-storchaka

serhiy-storchaka commented May 28, 2019

Copy link
Copy Markdown
Contributor Author

@shagren noticed that Path.cwd() / path does not work if path is a relative path on different disk.

@codecov

codecov Bot commented May 28, 2019

Copy link
Copy Markdown

Codecov Report

Merging #815 into master will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #815      +/-   ##
==========================================
+ Coverage   90.25%   90.34%   +0.08%     
==========================================
  Files          38       38              
  Lines        3347     3347              
  Branches      484      484              
==========================================
+ Hits         3021     3024       +3     
+ Misses        238      236       -2     
+ Partials       88       87       -1
Impacted Files Coverage Δ
neuromation/api/url_utils.py 87.71% <100%> (ø) ⬆️
neuromation/cli/main.py 60.73% <0%> (+1.57%) ⬆️

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 d5a04bd...8b0731b. Read the comment docs.

@codecov

codecov Bot commented May 28, 2019 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #815 into master will increase coverage by 0.44%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #815      +/-   ##
=========================================
+ Coverage   90.25%   90.7%   +0.44%     
=========================================
  Files          38      38              
  Lines        3347    3347              
  Branches      484     484              
=========================================
+ Hits         3021    3036      +15     
+ Misses        238     227      -11     
+ Partials       88      84       -4
Impacted Files Coverage Δ
neuromation/api/url_utils.py 92.98% <100%> (+5.26%) ⬆️
neuromation/cli/formatters/jobs.py 94.34% <0%> (+0.86%) ⬆️
neuromation/cli/utils.py 95.08% <0%> (+0.89%) ⬆️
neuromation/cli/main.py 63.35% <0%> (+4.18%) ⬆️

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 d5a04bd...ee9466e. Read the comment docs.

@serhiy-storchaka

serhiy-storchaka commented May 28, 2019

Copy link
Copy Markdown
Contributor Author

There is a known bug in the current implementation of Path.absolute() (see https://bugs.python.org/issue36305, python/cpython#12361).

Path.resolve() works correctly, but resolves also .., . and symlinks. If we want to preserve them we must use the trick: Path(path.anchor).resolve() / path.

shagren
shagren approved these changes May 28, 2019
View reviewed changes

@shagren shagren left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's do CI green again

@serhiy-storchaka
serhiy-storchaka merged commit cc9e552 into master May 28, 2019
@serhiy-storchaka
serhiy-storchaka deleted the test-windows-absolute-path branch May 28, 2019 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@shagren shagren shagren approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug Something isn't working platform

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@serhiy-storchaka @shagren @astaff