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

Bug with nextstrain update in >1.7 #37

Closed
trvrb opened this issue Dec 22, 2018 · 3 comments
Closed

Bug with nextstrain update in >1.7 #37

trvrb opened this issue Dec 22, 2018 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@trvrb
Copy link
Member

trvrb commented Dec 22, 2018

Calling nextstrain update in version 1.6.1 gets me:

Leda:mumps trvrb$ nextstrain update
A new version of nextstrain-cli, 1.7.1, is available!  You're running 1.6.1.

Upgrade by running:

    pip install --upgrade nextstrain-cli

Updating Docker image nextstrain/base…

Using default tag: latest
latest: Pulling from nextstrain/base
Digest: sha256:036a0368430163bfe76ee02cdd1318b9e3ba21ffbde13a26c1eddb05c9c20c75
Status: Downloaded newer image for nextstrain/base:latest

Pruning old copies of image…


Your images are up to date!

…but consider upgrading nextstrain-cli too, as noted above.

Everything working as it should be. However, running nextstrain update in 1.7.0 or 1.7.1 gets me:

Leda:mumps trvrb$ nextstrain update
nextstrain-cli is up to date!

Updating Docker image from nextstrain/base to nextstrain/base:build-20181222T010646Z…

build-20181222T010646Z: Pulling from nextstrain/base
Digest: sha256:036a0368430163bfe76ee02cdd1318b9e3ba21ffbde13a26c1eddb05c9c20c75
Status: Image is up to date for nextstrain/base:build-20181222T010646Z
Traceback (most recent call last):
  File "/Users/trvrb/.pyenv/versions/3.6.1/bin/nextstrain", line 11, in <module>
    sys.exit(main())
  File "/Users/trvrb/.pyenv/versions/3.6.1/lib/python3.6/site-packages/nextstrain/cli/__main__.py", line 10, in main
    return cli.run( argv[1:] )
  File "/Users/trvrb/.pyenv/versions/3.6.1/lib/python3.6/site-packages/nextstrain/cli/__init__.py", line 51, in run
    return opts.__command__.run(opts)
  File "/Users/trvrb/.pyenv/versions/3.6.1/lib/python3.6/site-packages/nextstrain/cli/command/update.py", line 27, in run
    for runner in all_runners
  File "/Users/trvrb/.pyenv/versions/3.6.1/lib/python3.6/site-packages/nextstrain/cli/command/update.py", line 27, in <listcomp>
    for runner in all_runners
  File "/Users/trvrb/.pyenv/versions/3.6.1/lib/python3.6/site-packages/nextstrain/cli/runner/docker.py", line 186, in update
    config.set("docker", "image", latest_image)
  File "/Users/trvrb/.pyenv/versions/3.6.1/lib/python3.6/site-packages/nextstrain/cli/config.py", line 75, in set
    save(config)
  File "/Users/trvrb/.pyenv/versions/3.6.1/lib/python3.6/site-packages/nextstrain/cli/config.py", line 43, in save
    with path.open(mode = "w", encoding = "utf-8") as file:
  File "/Users/trvrb/.pyenv/versions/3.6.1/lib/python3.6/pathlib.py", line 1164, in open
    opener=self._opener)
  File "/Users/trvrb/.pyenv/versions/3.6.1/lib/python3.6/pathlib.py", line 1018, in _opener
    return self._accessor.open(self, flags, mode)
  File "/Users/trvrb/.pyenv/versions/3.6.1/lib/python3.6/pathlib.py", line 390, in wrapped
    return strfunc(str(pathobj), *args)
FileNotFoundError: [Errno 2] No such file or directory: '/Users/trvrb/.nextstrain/config'
@trvrb trvrb added the bug Something isn't working label Dec 22, 2018
@tsibley
Copy link
Member

tsibley commented Dec 22, 2018

Ah, thanks for this. I'll fix it up when I have a chance. It looks like loading the config should guard against the file missing and return a default instead.

@tsibley
Copy link
Member

tsibley commented Dec 22, 2018

Hmm. I can't reproduce this on 1.7.1, and indeed the code already guards against the cases I thought you were hitting: https://github.com/nextstrain/cli/blob/master/nextstrain/cli/config.py#L36-L41

That error is what I'd expect if the /Users/trvrb/.nextstrain directory didn't exist by the time path.open was called, which would only happen if the mkdir() silently failed. I can't get it to do so.

@tsibley tsibley self-assigned this Dec 22, 2018
@tsibley
Copy link
Member

tsibley commented Dec 22, 2018

Ah ha! I think you're hitting this Python bug in path.resolve(), which was fixed first in 3.6.2 (you have 3.6.1) and noted with the following changelog entry:

path.resolve(strict=False) no longer cuts the path after the first element not present in the filesystem. Patch by Antoine Pietri.

Note that strict=False is the default.

Indeed, this description of the effect of the bug matches what we're seeing:

For what it's worth, this bug was the cause of an important downtime in our organization, because someone needed to normalize a path that was later passed to a .mkdir(parents=True), and it was, in some cases, silently doing the mkdir of a wrong path instead of creating all the parents.

This means that the mkdir() in our code on your computer was happening against /Users/trvrb instead of /Users/trvrb/.nextstrain! which matches my diagnosis above.

I'll add a workaround in our code next week. In the meantime, you can mkdir ~/.nextstrain and move on with your life if you haven't already.

On top of all this, I also now see that the default behaviour for path.resolve() changed between 3.5 and 3.6, with no way at all to ask for 3.6's default behaviour (which we want) on 3.5. I'd thought the defaults matched. I'll address that as well.

tsibley added a commit that referenced this issue Dec 27, 2018
  • 3.5 is the earliest version with which we aim to maintain compat
  • a change in default behaviour from 3.6.1 → 3.6.2 caused issue #37
  • 3.7 has been released
  • 3.8 is the new dev focus
tsibley added a commit that referenced this issue Dec 27, 2018
  • 3.5 is the earliest version with which we aim to maintain compat
  • a change in default behaviour from 3.6.1 → 3.6.2 caused issue #37
  • 3.7 has been released
  • 3.8 is the new dev focus

The use of an explicit build matrix is because only trusty contains
3.6.1 and only xenial contains 3.7 and 3.8-dev.  The latter is the newer
dist, so we use that for 3.5 and 3.6 as well.
tsibley added a commit that referenced this issue Dec 27, 2018
  • 3.5 is the earliest version with which we aim to maintain compat
  • a change in default behaviour from 3.6.1 → 3.6.2 caused issue #37
  • 3.7 has been released
  • 3.8 is the new dev focus

The use of an explicit build matrix is because only trusty contains
3.6.1 and only xenial contains 3.7 and 3.8-dev.  The latter is the newer
dist, so we use that for 3.5 and 3.6 as well.
tsibley added a commit that referenced this issue Dec 27, 2018
This should reveal failures on 3.5 and 3.6.1.

  • 3.5 is the earliest version with which we aim to maintain compat,
    but it's broken as noticed in issue #37

  • 3.6.1 has a different default behaviour than 3.6.2+ for
    Path.resolve(), causing issue #37

  • 3.7 has been released
  • 3.8 is the new dev focus

The use of an explicit build matrix is because only trusty contains
3.6.1 and only xenial contains 3.7 and 3.8-dev.  The latter is the newer
dist, so we use that for 3.5 and 3.6 as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants