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

RF: Demote symlinked CWD related log message to debug-level #4426

Merged
merged 2 commits into from Apr 20, 2020

Conversation

mih
Copy link
Member

@mih mih commented Apr 19, 2020

This is the message:

[WARNING] realpath of PWD=</some/path/to/symlinked/file> is </resolved/path/to/symlinked/file> whenever os.getcwd()=<path/to/dataset>

Questions about it have motivated a dedicated "please ignore it" PR
in the handbook: datalad-handbook/book#438

I think this message is pointless (but still unconditionally shown) if
everything works. When something doesn't work, it is likely caused by
a bug in DataLad. Hence there is no need to bother users with this
warning that they can or should do nothing about.

Fixes gh-4425

mih and others added 2 commits Apr 19, 2020
This is the message:

[WARNING] realpath of PWD=</some/path/to/symlinked/file> is </resolved/path/to/symlinked/file> whenever os.getcwd()=<path/to/dataset>

Questions about it have motivated a dedicated "please ignore it" PR
in the handbook: datalad-handbook/book#438

I think this message is pointless (but still unconditionally shown) if
everything works. When something doesn't work, it is likely caused by
a bug in DataLad. Hence there is no need to bother user with this
warning that they can or should do nothing about.
@kyleam
Copy link
Collaborator

kyleam commented Apr 20, 2020

I think demoting this would be good, but perhaps I'm not thinking of some nasty case that makes the noise/user confusion worth it in the other cases.

In gh-2910, @yarikoptic wrote

in getpwd verify that envvar PWD and os.getpwd() point to the same directory (after realpath'ing it), and if not - return os.getpwd() (with some warning?).

The last bit suggests he was on the fence about the warning at the time. Not sure if he ended up with a clear preference after working on the PR.

@codecov
Copy link

codecov bot commented Apr 20, 2020

Codecov Report

Merging #4426 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4426      +/-   ##
==========================================
+ Coverage   88.91%   88.92%   +0.01%     
==========================================
  Files         285      285              
  Lines       37765    37814      +49     
==========================================
+ Hits        33578    33626      +48     
- Misses       4187     4188       +1     
Impacted Files Coverage Δ
datalad/tests/test_utils.py 95.94% <100.00%> (ø)
datalad/utils.py 83.96% <100.00%> (ø)
datalad/support/tests/test_annexrepo.py 95.29% <0.00%> (-0.03%) ⬇️
datalad/distribution/dataset.py 94.14% <0.00%> (ø)
datalad/interface/common_opts.py 100.00% <0.00%> (ø)
datalad/interface/tests/test_utils.py 100.00% <0.00%> (ø)
datalad/tests/test_config.py 99.22% <0.00%> (+0.01%) ⬆️
datalad/support/annexrepo.py 86.37% <0.00%> (+0.02%) ⬆️
datalad/interface/utils.py 94.11% <0.00%> (+0.02%) ⬆️
datalad/support/network.py 86.66% <0.00%> (+0.03%) ⬆️
... and 2 more

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 23d7683...4022859. Read the comment docs.

@yarikoptic
Copy link
Member

yarikoptic commented Apr 20, 2020

also it was added when we largely returned absolute paths, now we more often return relative (too relative IMHO, see #3442) so indeed there is less sources of confusion we tried to avoid by issuing this warning. So FWIW I am ok to lower it to DEBUG thus let's go

@yarikoptic yarikoptic merged commit dc7100f into datalad:master Apr 20, 2020
12 checks passed
@yarikoptic yarikoptic added this to the 0.13.0 milestone Apr 23, 2020
@mih mih deleted the bf-4425 branch Jul 19, 2022
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