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

[WIP] Version bump Conda from 4.6.14 to 4.7.5 #719

Merged
merged 13 commits into from Jul 12, 2019

Conversation

@davidrpugh
Copy link
Contributor

davidrpugh commented Jun 26, 2019

Conda 4.7.5 was released yesterday. This PR bumps the Conda version from 4.6.14 to 4.7.5. Note that a new version of Miniconda has not yet been released. I will bump that version number as well if a newer version is released before this PR is merged.

This is my first PR and I did my best to identify all possible locations where the Conda version number was explicitly specified. I only found two in repo2docker/buildpacks/conda directory.

@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jun 26, 2019

I am a bit stuck on a failing Travis build. Here is the relevant snippet from the Travis logs...

Step 41/48 : RUN conda env update -p ${NB_PYTHON_PREFIX} -f "environment.yml" && conda clean --all -f -y && conda list -p ${NB_PYTHON_PREFIX}
 ---> Running in 3ff1f1fc4e1b
Collecting package metadata (repodata.json): ...working... done
Solving environment: ...working... failed
ResolvePackageNotFound: 
  - xeus-cling=0.4.5 -> util-linux
  - xeus-cling=0.4.5 -> xeus[version='>=0.13.0,<0.14']
Removing intermediate container 3ff1f1fc4e1b
The command '/bin/sh -c conda env update -p ${NB_PYTHON_PREFIX} -f "environment.yml" && conda clean --all -f -y && conda list -p ${NB_PYTHON_PREFIX}' returned a non-zero code: 1

And here is the failing test in test/external/reproductions.repos.yml...

# Test that custom channels and downgrades don't
# get blocked by pinning
- name: Xeus-Cling
  url: https://github.com/QuantStack/xeus-cling
  ref: 0.4.5
  verify: jupyter kernelspec list

I am going to try adding the free channel back into the defaults meta-channel to see if this solves the problem. Removing the free channel was one of the major optimizations included in conda 4.7 so this change might partially negate the faster builds that conda 4.7 would provide.

@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jun 26, 2019

So, adding the conda config --system --set restore_free_channel true causes pretty much all of the builds to fail. However I think this is due to the fact the the the underlying version of Miniconda is still 4.6.14 < 4.7 which is why the restore_free_channel is not recognized as a primitive config parameter.

I think that this PR will need to marinate until a new version of Miniconda is released...

@davidrpugh davidrpugh changed the title Version bump Conda from 4.6.14 to 4.7.5 [WIP] [WIP] Version bump Conda from 4.6.14 to 4.7.5 Jun 27, 2019
@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jun 27, 2019

@minrk suggestion fixes all the test failures except the original xeus-cling failure mentioned further up the thread. Looks like I was wrong about the source of the problem: we don't need to include the free channel after all (which is good because excluding free from defaults is one of the optimizations provided in Conda 4.7.5); however now I am a bit stuck as I am not sure to root cause of why the xeus-cling test is failing.

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Jun 27, 2019

I think the repo itself was broken due to a recent update, but appears to be fixed now. So just a coincidence that this PR was failing. I just restarted that test, so I think it will pass this time.

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jun 27, 2019

I thought we build a specific tag/branch of the xeus-cling repo so that it wouldn't cause random breakage. Worth investigating if we really build the tag?

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Jun 27, 2019

I think there is a relevant change in conda 3.7, since building this repo with master doesn't pick up 0.19.3. Perhaps there is a change in channel priority resolution that we need to investigate that could be a bug or a change in defaults we need to check.

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jun 27, 2019

Not sure I fully understand the error message from conda but if I look at https://anaconda.org/conda-forge/xeus-cling/files there doesn't seem to be a xeus-cling=0.4.5. This makes me think that the version doesn't exist (any more). Time to update our test to a newer version?

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Jun 27, 2019

0.4.5 is on the QuantStack channel

@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jun 27, 2019

I think there is a relevant change in conda 3.7, since building this repo with master doesn't pick up 0.19.3. Perhaps there is a change in channel priority resolution that we need to investigate that could be a bug or a change in defaults we need to check.

I was aware that the composition of the defaults meta-channel change with conda 4.7 and was why I thought restoring the free channel to defaults might have caused the test to pass. But this had no effect on the failing test.

There were also some changes to channel priority that might be relevant but from my reading of the docs the default channel_priority=flexible in conda 4.7 should behave the same as the default channel_priority=true in conda 4.6. The default value will change to `channel_priority=strict' in conda 5.0 which will lead to performance benefits (but might cause more of our tests to fail).

@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jun 29, 2019

@betatim and @minrk I tried explicitly setting channel_priority "strict" in the conda config file to see what impact that would have. It didn't solve the problem with the Xeus-Cling test but it did identify some other issues that we might want to deal with now as strict channel priority will become the default behavior in Conda 5.

The Xeus-Cling test is still generating the same ResolvePackageNotFound error which is odd as best that I can tell the enviornment.yml file for the 0.4.5 release includes the QuantStack channel as the top priority channel. I did however notice that the environment.yml file for the most recent release does not include the QuantStack channel. How is the environment.yml file each test constructed? Using the environment.yml file from the repo associated with a particular release?

The other test failures are from UnsatisfiableError and seem to be generated by repos where the conda-forge/label/broken channel contains packages that are needed to build an environment but these packages exist in higher priority channels. Enabling "strict" channel priority doesn't allow conda to pull packages from lower priority channels if they exist in higher priority channels. Is there a good reason why conda-forge/label/broken should be given lowest priority?

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jun 30, 2019

How is the environment.yml file each test constructed? Using the environment.yml file from the repo associated with a particular release?

For these tests we run the command a user would run repo2docker --ref <tag/ref/commit> <path-to-repo> so you get exactly the version of the repo that corresponds to the tag.

Is there a good reason why conda-forge/label/broken should be given lowest priority?

The idea is to let repositories that install package some-package==1.2.3 (which isn't marked as broken) to continue to build even after that package has been marked as broken. With strict on that won't work any more though as packages will always be present in a higher priority channel. We can't make broken have a higher priority because then you couldn't install any packages which have a single broken package.

I think when we do activate strict we should update the xeus-cling test as I think we couldn't build that repo any more as the xeus-cling package is now in the conda-forge channel so with strict on we won't be able to access the 0.4.5 version any more.

@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jun 30, 2019

@betatim thanks for the clarifications. It seems clear that we cannot use strict channel priority and have the conda-forge/label/broken channel the lowest priority channel. What it we made conda-forge/label/broken the highest priority channel?

I still don't understand why the Xeus-Cling test is failing as the repo with tag 0.4.5 has an environment file that includes the QuantStack channel which still has 0.4.5 of xeus-cling as the highest priority channel. Unless this priority is being over-ridden by the fact that we explicitly added the conda-forge channel to the system default channels in the miniconda-install.sh script.

# Allow easy direct installs from conda forge
conda config --system --add channels conda-forge

I am not sure how channel priority is determined if conda-forge is included in the default available channels and also included in the user specified list of channels in the environment file. Should we be adding conda-forge as to the list of default channels? The comment suggests that this setting is purely for convenience. Perhaps it would be better to leave which channels are available and the exact priority completely to the users? I will go ahead and remove the above line and see what happens...

pughdr added 2 commits Jun 30, 2019
…l/broken channel as low priority channel
@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jun 30, 2019

So removing...

# Allow easy direct installs from conda forge
conda config --system --add channels conda-forge

...did not impact anything. I would advocate leaving it out just to make it explicit that the user should explicitly include the conda-forge channel if they want to use it.

Restoring channel_priority "flexible" allows all tests to pass expect the Xeus-Cling test. I don't understand why the Xeus-Cling test continues to fail. It seems like the QuantStack channel is not being included in the environment.yml file for the repo. I must be missing something.

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Jul 2, 2019

Please don't remove conda-forge. That's a deliberate choice, and where ~all of the packages in the base env come from.

@minrk

This comment has been minimized.

Copy link
Member

minrk commented Jul 2, 2019

From channel priority docs:

# # channel_priority (ChannelPriority)
# #   Accepts values of 'strict', 'flexible', and 'disabled'. The default
# #   value is 'flexible'. With strict channel priority, packages in lower
# #   priority channels are not considered if a package with the same name
# #   appears in a higher priority channel. With flexible channel priority,
# #   the solver may reach into lower priority channels to fulfill
# #   dependencies, rather than raising an unsatisfiable error. With channel
# #   priority disabled, package version takes precedence, and the
# #   configured priority of channels is used only to break ties. In

Which means that strict is too strict for us. It doesn't allow the solver to fallback on older versions or explicit pins. It essentially makes frozen envs impossible, so I think strict channel priority can never be the right choice in repo2docker.

Some cases where strict priority would do the wrong thing for us:

  • package only in defaults with a dependency packaged also in conda-forge, build the build and/or version in conda-forge is incompatible
  • conda env export that includes packages in defaults and/or later marked broken (this is the use case for which we allow broken)
@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jul 2, 2019

Please don't remove conda-forge. That's a deliberate choice, and where ~all of the packages in the base env come from.

I have re-added it. But am curious to know how the inclusion of conda-forge in the .condarc file interact with a user's environment.yml file channels (and priorities)?

Currently looks like conda-forge is given priority over defaults in the .condarc file. If a user doesn't specify channels in the environment.yml file then they (probably) pull from the defaults channel when working locally (unless user has also given conda-forge priority in their local .condarc) but then user environment would pull from conda-forge when using repo2docker? Perhaps I am missing something.

From what I can gather from the docs. It seem like when a user specifies channels, then these channels are given priority over channels specified in .condarc (but channels in .condarc are still searched unless --override-channels option is used).

@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jul 2, 2019

Which means that strict is too strict for us. It doesn't allow the solver to fallback on older versions or explicit pins. It essentially makes frozen envs impossible, so I think strict channel priority can never be the right choice in repo2docker.

Some cases where strict priority would do the wrong thing for us:

* package only in defaults with a dependency packaged also in conda-forge, build the build and/or version in conda-forge is incompatible

* `conda env export` that includes packages in defaults and/or later marked broken (this is the use case for which we allow broken)

Yes I agree. I have added a line to explicitly set channel priority to be flexible to future-proof against conda switching to strict as the default channel priority.

@ocefpaf

This comment has been minimized.

Copy link

ocefpaf commented Jul 2, 2019

Which means that strict is too strict for us. It doesn't allow the solver to fallback on older versions or explicit pins.

@minrk we should probably check why those exactly pins are needed and fix the packages in conda-forge to accommodate strict in repo2docker. I'm happy to work on the conda-forge side to make this happen.

It essentially makes frozen envs impossible

Not sure that is true. As long as we have the set of packages you need it is possible to do frozen envs.

so I think strict channel priority can never be the right choice in repo2docker.

Hope you change your mind. Using strict at the base image really improves speed of the envs we create and their stability.

Some cases where strict priority would do the wrong thing for us:

  • package only in defaults with a dependency packaged also in conda-forge, build the build and/or version in conda-forge is incompatible

defaults is designed to be a subset of conda-forge with some exceptions (like compilers), so if a package is missing in conda-forge we should add it. Note that defaults is also the canonical provider of compilers, so those are not needed in conda-forge b/c the scenario you mention here won't happen.

  • conda env export that includes packages in defaults and/or later marked broken (this is the use case for which we allow broken)

I don't understand the use conda conda export in repo2docker but allowing broken is scary! Things here are working probably b/c of luck and not by design.

I'm not familiar with repo2docker but could we enable strict at later time? After the base repo2docker environment was already created? That way users relying on this can have strict enabled for their environments later on.

Pinging @scopatz here who may help me phrase these conda-forge usage best practices in an better way.

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jul 2, 2019

Please let's move the discussion of strict to a new issue. We don't need a decision on it to merge (or not) this PR. I think it is an interesting topic to discuss, it might yield a large number of ideas and new knowledge. All reasons to take it to its dedicated thread.

Given that bumping conda versions has already generated ~20 messages I think we should try and land this PR with the minimal set of changes. Moving other nice-to-haves to new PRs. Divide and conquer style.

Thank you :)

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jul 2, 2019

There is a new failure in the tests which will also happen on master (I think). While trying to build the Zenodo record for https://github.com/mbcxqcw2/EEModel/blob/v1.03/requirements.txt, we get this:

      File "/srv/conda/envs/kernel/lib/python2.7/site-packages/setuptools/sandbox.py", line 45, in _execfile
        exec(code, globals, locals)
      File "/tmp/easy_install-VBFt7K/numpy-1.17.0rc1/setup.py", line 31, in <module>

    RuntimeError: Python version >= 3.5 required.
    ----------------------------------------
ERROR: Command "python setup.py egg_info" failed with error code 1 in /tmp/pip-install-sgij5M/matplotlib/

while trying to install matplotlib-1.5.1. It is odd that this is trying to install numpy 1.17 from source instead of 1.10.4 which is what is specified in requirements.txt :-/ The fact that this is appearing now probably means something in the outside world changed (package becoming unavailable or some such).

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jul 2, 2019

https://pypi.org/project/numpy/#history shows that numpy 1.17 was released at the end of June. Don't ask me why we are now trying to install it though :)

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jul 3, 2019

I've created #729 to investigate why this happens. All help and ideas welcome as this blocks all PRs. I don't have a good idea why we run into this.

@scopatz

This comment has been minimized.

Copy link

scopatz commented Jul 3, 2019

@betatim is there a new issue to discussion the strict and broken issue?

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jul 4, 2019

is there a new issue to discussion the strict and broken issue?

There is #731

pughdr
@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jul 9, 2019

@betatim I pulled in the updates from master so that the zenodo test now passes. Still not sure what to do about the remaining failure.

@minrk minrk referenced this pull request Jul 9, 2019
@minrk

This comment has been minimized.

Copy link
Member

minrk commented Jul 9, 2019

Re-adding the free channel should fix that test, but it seems to run into another unsatisfiable error:

UnsatisfiableError: The following specifications were found to be incompatible with each other:

  - xeus-cling=0.4.5 -> xeus[version='>=0.13.0,<0.14'] -> *[track_features=cling]

Bumping xeus-cling to a more recent tag (#735) does get the test passing with no more changes in this PR. The question becomes: do we enable free by default or not? I'm inclined to say no, as it's what's most likely to behave the same as home-user-conda, but I'm not certain. Enabling free would be the minimizing changes over time, even if those changes are mostly good. Users can always opt-in to free in an environment.yml.

@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jul 9, 2019

Re-adding the free channel should fix that test, but it seems to run into another unsatisfiable error:

Yes I also expected that this would fix the issue. I tried adding the suggesting setting to re-add the free channel in a previous commit and got the same error as you.

Bumping xeus-cling to a more recent tag (#735) does get the test passing with no more changes in this PR.

Is this your proposed solution? If so, it is fine by me as I don't see any other alternative on our end. Should I bump the version myself? If so, what version number would you prefer?

The question becomes: do we enable free by default or not? I'm inclined to say no, as it's what's most likely to behave the same as home-user-conda, but I'm not certain. Enabling free would be the minimizing changes over time, even if those changes are mostly good. Users can always opt-in to free in an environment.yml.

I agree that we should not enable free by default. Leaving free disabled matches the default behavior for all users who keep their local conda install up to date and leaving free disabled also yields significant speed-ups when solving/building environments. As you say users can always explicitly add the free channel if they wish.

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jul 10, 2019

Can you rebase this on the current master again now that #735 got merged please?

pughdr
@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jul 10, 2019

Done. Hopefully everything will now pass...

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jul 12, 2019

LGTM. Thanks for the patience!

@betatim betatim merged commit 399b339 into jupyter:master Jul 12, 2019
3 checks passed
3 checks passed
ci/circleci: build_docs Your tests passed on CircleCI!
Details
ci/dockercloud Your tests passed in Docker Cloud
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@davidrpugh

This comment has been minimized.

Copy link
Contributor Author

davidrpugh commented Jul 12, 2019

Thanks! Looks like we merged just in time for a new conda release...

https://github.com/conda/conda/releases

...4.7.6 is out. What are your thoughts? Would you prefer to wait for the next minor release or would you prefer to update patches as they come out?

@betatim

This comment has been minimized.

Copy link
Member

betatim commented Jul 13, 2019

I'd wait a bit to give the dust a chance to settle. 4.7.7 got release just now ;)

@davidrpugh davidrpugh deleted the kaust-vislab:conda-4.7.5 branch Jul 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.