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.

Sign up for GitHub

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

Jump to bottom

Add support for libmamba integrations (clean up poc-libsolv) #11193

Merged
merged 41 commits into from
Feb 25, 2022
Merged

Add support for libmamba integrations (clean up poc-libsolv) #11193

merged 41 commits into from
Feb 25, 2022

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Feb 2, 2022 •
edited

Supersedes #10881

We will cleanup the long-lived poc-libsolv branch here to provide a clearer diff with no formatting:

Implemented changes (wip):

  • Add mamba dependencies to dev requirements (incl. Docker images) (dd847c2)
  • Add solver_logic Enum and _get_solver_logic() forwarder (09c10b9)
  • Tune UnsatisfiableError exceptions (082ff64)
  • Rework logging to allow file logs with debug level (0808ce2, e3bab36)
  • Rework testing infrastructure to support downstream packages (includes fixes to original helpers for local channel serialization instead of in-memory cache override) (954e4a0, fa4d5b8, b55085b, 86dafe3, 0c72113, 0f9eb57, a255c1d)

Test changes:

  • change timestamp in test data so it's one SECOND older (not just one ms) (6a5f3aa)
  • Change expected logging levels (part of 0808ce2)

Other small fixes:

  • Fix URL encoding / decoding (otherwise mamba fails to load local channels) (e7a4d03)
  • add fresh_context contextmanager (needed in some downstream tests) (a025443)
  • resolve: move specs conversion before logging (the logger is enabled for debug for our file logs, so this subtle bug is uncovered) (0df41a4)
  • add context.override contextmanager (c8ffe7a)
  • add str to auxlib.Null for cleaner depiction in logs (c0c2118)
  • default to REPODATA_FN is passed argument is falsey (1ad4cfb)
  • fix logging typo (unimportant) (29372e5)
  • temporary workaround for .conda_build_form() deficiencies described in MatchSpec.conda_build_form() expects version to be set #11200 (f26c643)

@jaimergp jaimergp added the solver::libmamba pertains to the libmamba solver label Feb 2, 2022
@anaconda-issue-bot anaconda-issue-bot added the cla-signed [bot] added once the contributor has signed the CLA label Feb 2, 2022
@jaimergp jaimergp marked this pull request as ready for review February 7, 2022 17:15
@jaimergp jaimergp requested a review from a team as a code owner February 7, 2022 17:15
jaimergp
jaimergp commented Feb 7, 2022
View reviewed changes
.github/workflows/ci.yml Outdated Show resolved Hide resolved
jaimergp
jaimergp commented Feb 7, 2022
View reviewed changes
conda/common/io.py Outdated Show resolved Hide resolved
jaimergp
jaimergp commented Feb 7, 2022
View reviewed changes
conda/exceptions.py Outdated Show resolved Hide resolved
@jaimergp
Copy link
Contributor Author

jaimergp commented Feb 7, 2022

I think this is ready to go @jezdez! I have left some comments already in the interesting bits. Changes have been reduced to 50% more or less, but still most of that is moving pieces of code round (populating conda.testing, mainly).

@jaimergp jaimergp added the build::review trigger a build for this PR label Feb 7, 2022
@conda-bot conda-bot removed the build::review trigger a build for this PR label Feb 7, 2022
@conda-bot
Copy link
Contributor

conda-bot commented Feb 7, 2022 •
edited

Review build status

The workflow for building and uploading a canary release for conda with the label conda-conda-pr-11193 in channel conda-canary was started by @jaimergp!

Once it's done, use this command to try it out in a new conda environment:

conda install -c conda-canary/label/conda-conda-pr-11193 conda

@jezdez
Copy link
Member

jezdez commented Feb 7, 2022

This needs a pass over the linter in conda/testing for some reason, to get the linelength correctly mostly. Other than that this looks good!

@jaimergp
Copy link
Contributor Author

jaimergp commented Feb 7, 2022

I intentionally left the line length checks outside of the initial review scope to make the diff less noisy. Happy to apply those fixes once you give me the green light!

jezdez
jezdez approved these changes Feb 8, 2022
View reviewed changes
conda/base/context.py Outdated Show resolved Hide resolved
@jaimergp jaimergp added the build::review trigger a build for this PR label Feb 8, 2022
@conda-bot conda-bot removed the build::review trigger a build for this PR label Feb 8, 2022
@jezdez jezdez mentioned this pull request Feb 8, 2022
Closed
@bgruening bgruening mentioned this pull request Feb 9, 2022
Closed
@jaimergp
Copy link
Contributor Author

I think we are ready here too, @jezdez

@jaimergp jaimergp added the build::review trigger a build for this PR label Feb 14, 2022
@conda-bot conda-bot removed the build::review trigger a build for this PR label Feb 14, 2022
@jezdez jezdez mentioned this pull request Feb 25, 2022
Closed
jezdez
jezdez reviewed Feb 25, 2022
View reviewed changes
tests/requirements.txt Outdated Show resolved Hide resolved
@jaimergp
Copy link
Contributor Author

Oops, sorry, I was reviewing an old commit!

@jezdez jezdez self-assigned this Feb 25, 2022
@jezdez
Copy link
Member

jezdez commented Feb 25, 2022

It's time.

@jezdez jezdez merged commit c92b206 into master Feb 25, 2022
@jezdez jezdez mentioned this pull request Feb 25, 2022
Merged
@jezdez jezdez deleted the libmamba branch March 10, 2022 12:06
@jezdez jezdez restored the libmamba branch March 10, 2022 12:06
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Mar 19, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 19, 2023
@jezdez jezdez deleted the libmamba branch May 16, 2023 15:12
mbargull
mbargull reviewed Jan 30, 2024
View reviewed changes
conda/gateways/logging.py
conda_logger.setLevel(level)
conda_logger.propagate = True # let root logger's handler format/output message
conda_logger = getLogger("conda")
conda_logger.setLevel(logging.NOTSET)
Copy link
Member

Choose a reason for hiding this comment

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

This and the equivalent change in conda.common.io causes log.isEnabledFor(DEBUG)-guarded code to be run when not intended.
xref: #13541

Copy link
Member

Choose a reason for hiding this comment

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

Nice find!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oopsie 😬

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jezdez jezdez jezdez approved these changes

@mbargull mbargull mbargull left review comments

Assignees

@jezdez jezdez

Labels
cla-signed [bot] added once the contributor has signed the CLA locked [bot] locked due to inactivity solver::libmamba pertains to the libmamba solver
Projects
None yet
Milestone
Libmamba solver release (experimental)
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@jaimergp @conda-bot @jezdez @mbargull @anaconda-issue-bot