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

Stop importing from conda/exports.py #13869

Merged
merged 8 commits into from May 1, 2024

Conversation

beeankha
Copy link
Contributor

@beeankha beeankha commented Apr 26, 2024

While investigating a fix for #13860, an instance of reset_context() was found in conda/exports.py; in order to avoid resetting the context unintentionally, as well as to avoid utilizing the conda/exports.py module (which is meant to be used by conda-build vs conda), this PR attempts to remove/edit/update any import statement in the conda codebase that is importing from conda/exports.py.

Certain cases were left alone, such as the imports from exports.py in tests/test_create.py in test_list_with_pip_no_binary and test_rm_rf, since those are specifically related to regression testing.

Also related to the following deprecation issue:

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Apr 26, 2024
Copy link

codspeed-hq bot commented Apr 26, 2024

CodSpeed Performance Report

Merging #13869 will not alter performance

Comparing beeankha:remove-exports-imports (9721a24) with main (ad61575)

Summary

✅ 21 untouched benchmarks

@beeankha beeankha marked this pull request as ready for review April 29, 2024 15:35
@beeankha beeankha requested a review from a team as a code owner April 29, 2024 15:35
Copy link
Contributor

@schuylermartin45 schuylermartin45 left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me, but I am not familiar enough with the implications of this change.

@jezdez
Copy link
Member

jezdez commented Apr 29, 2024

@beeankha The description of the PR doesn't match the content of the PR, since it changes only things in conda.plan, but not conda.exports. Could we update that somehow?

@jezdez jezdez self-requested a review April 29, 2024 19:51
@beeankha beeankha changed the title Remove imports from conda/exports.py Stop importing from conda/exports.py Apr 29, 2024
@beeankha
Copy link
Contributor Author

@beeankha The description of the PR doesn't match the content of the PR, since it changes only things in conda.plan, but not conda.exports. Could we update that somehow?

I agree that the wording is confusing; I meant we want to remove any import statements that are importing anything from the conda/exports.py module. I updated the title of this PR, hopefully that helps a bit. I'll see if I can edit the description to be a bit more clear.

jezdez
jezdez previously approved these changes Apr 29, 2024
kenodegard
kenodegard previously approved these changes May 1, 2024
@kenodegard kenodegard changed the base branch from main to 24.5.x May 1, 2024 19:52
@kenodegard kenodegard dismissed stale reviews from jezdez and themself May 1, 2024 19:52

The base branch was changed.

@kenodegard kenodegard merged commit 7f4ddde into conda:24.5.x May 1, 2024
66 checks passed
@kenodegard kenodegard mentioned this pull request May 1, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed [bot] added once the contributor has signed the CLA
Projects
Status: 🏁 Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants