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

Prepare deprecation of full index and action usage #5152

Merged
merged 4 commits into from
Jan 31, 2024

Conversation

mbargull
Copy link
Member

Description

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Jan 26, 2024
@mbargull mbargull force-pushed the deprecate-index-and-actions branch 9 times, most recently from 7ea64c6 to 17a7d9a Compare January 26, 2024 17:21
@mbargull
Copy link
Member Author

@kenodegard, @jezdez, I noticed we need to deprecate some more things when we replace the conda.core.get_index and install_actions usage.
This is based on gh-5151 and gh-5074, hence the large overall changeset.
The actual changes are only in commit 7ea64c6 .

What this does overall is:

  1. Mark channel data functionality in conda_build.index (now in conda-index) as deprecated since it requires iterating over the full index.
  2. Mark actions-related arguments as deprecated since actions is now always empty or a {PREFIX_ACTION: str, LINK_ACTION: list[PackageRecord]} dictionary for which the prefix is redundant because the call sites always have the prefix already which in turn means actions is really now equivalent to a list of PackageRecord.
    We want to remove the install_actions functionality which manually overrides the solver's index with the full index.
    As such, this PR prepares the functions and their arguments to just handle lists of PackageRecord so we can do away with all the legacy actions stuff in the next version.

Comment on lines +424 to 427
@deprecated(
"24.1.0", "24.5.0", addendum="Use `conda_index._apply_instructions` instead."
)
def _apply_instructions(subdir, repodata, instructions):
Copy link
Member Author

Choose a reason for hiding this comment

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

I marked this explicitly as deprecated since I know there are some conda-forge scripts that use _apply_instructions.
(We probably shouldn't to continue to use the underscore-prefixed function from conda-idex either. But this way we can at least mark it for removal here.)

I assume we don't have to mark every single underscore-prefixed symbol here separately, right?!

Copy link
Member

Choose a reason for hiding this comment

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

Every function that we know is used and plan to remove, independent whether it's prefixed with an underscore or not, is going to need the decorator.

Remember Hyrum's Law, everything is API in the eye of the user, we can only manage the way it's being used and informing the users about our intentions, is good engineering practice.

Copy link
Member Author

Choose a reason for hiding this comment

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

Every function that we know is used [...]

The problem is, we of course don't know which are used in the wild...
I've now just plonked the decorator on everything in conda_build.index 🤷.

(If anyone knows a convenient way to really handle API<->internals definition in Python which we could use for future modules/packages, I'm all ears. Python, since it's one of those "everything is public and dynamic" languages, is just awful when Mr. Wright et al. point out these issues...)

conda_build/environ.py Outdated Show resolved Hide resolved
@mbargull mbargull force-pushed the deprecate-index-and-actions branch from 17a7d9a to a93e5b3 Compare January 27, 2024 00:04
@mbargull mbargull marked this pull request as ready for review January 27, 2024 00:05
@dholth
Copy link
Contributor

dholth commented Jan 29, 2024

Nice, thanks for working on the index feature.

Optional channeldata in standalone conda-index https://github.com/conda/conda-index/blob/main/conda_index/cli/__init__.py#L134-L141

conda_build/environ.py Outdated Show resolved Hide resolved
Comment on lines +424 to 427
@deprecated(
"24.1.0", "24.5.0", addendum="Use `conda_index._apply_instructions` instead."
)
def _apply_instructions(subdir, repodata, instructions):
Copy link
Member

Choose a reason for hiding this comment

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

Every function that we know is used and plan to remove, independent whether it's prefixed with an underscore or not, is going to need the decorator.

Remember Hyrum's Law, everything is API in the eye of the user, we can only manage the way it's being used and informing the users about our intentions, is good engineering practice.

@mbargull mbargull force-pushed the deprecate-index-and-actions branch from 05a6ef4 to 70dfbf7 Compare January 30, 2024 14:52
@mbargull mbargull force-pushed the deprecate-index-and-actions branch from 70dfbf7 to 4c8a732 Compare January 30, 2024 14:56
@mbargull mbargull mentioned this pull request Jan 30, 2024
43 tasks
@kenodegard kenodegard requested review from dholth and jezdez January 30, 2024 15:20
@mbargull
Copy link
Member Author

Needs news/CHANGELOG.md entries.
I'm afk for a bit, so feel free to add things at will.

kenodegard
kenodegard previously approved these changes Jan 31, 2024
@mbargull
Copy link
Member Author

Thanks for the updates and reviews!
I'm out for today -- GL HF on releasing! 🤞

@jezdez jezdez merged commit 7dbf29e into conda:24.1.x Jan 31, 2024
26 checks passed
@dholth dholth mentioned this pull request Feb 28, 2024
2 tasks
mbargull added a commit to mbargull-feedstocks/conda-forge-repodata-patches-feedstock that referenced this pull request Mar 16, 2024
conda_build.index._apply_instructions is marked for deprecation.
refs:
- conda/conda-build#5152 (comment)
- conda/conda-build#5226

Signed-off-by: Marcel Bargull <[email protected]>
mbargull added a commit to mbargull/cdt-builds that referenced this pull request Mar 16, 2024
conda_build.conda_interface.get_index (which is conda.exports.get_index)
is using the conda.models.Dist -> conda.models.records.PackageRecord
mapping. Dist class is legacy code that's being phased out, so avoid it.

refs:
- conda/conda-build#5152
- conda/conda-build#5222

Signed-off-by: Marcel Bargull <[email protected]>
mbargull added a commit to mbargull/cdt-builds that referenced this pull request Mar 16, 2024
conda_build.conda_interface is being deprecated.

conda_build.conda_interface.get_index (which is conda.exports.get_index)
is using the conda.models.Dist -> conda.models.records.PackageRecord
mapping. Dist class is legacy code that's being phased out, so avoid it.

refs:
- conda/conda-build#5152
- conda/conda-build#5222

Signed-off-by: Marcel Bargull <[email protected]>
mbargull added a commit to mbargull/conda-forge-webservices that referenced this pull request Mar 16, 2024
conda_build.conda_interface is being deprecated.

conda_build.conda_interface.get_index (which is conda.exports.get_index)
is using the conda.models.Dist -> conda.models.records.PackageRecord
mapping. Dist class is legacy code that's being phased out, so avoid it.

refs:
- conda/conda-build#5152
- conda/conda-build#5222

Signed-off-by: Marcel Bargull <[email protected]>
mbargull added a commit to mbargull/staged-recipes that referenced this pull request Mar 16, 2024
conda_build.conda_interface is being deprecated.

conda_build.conda_interface.get_index (which is conda.exports.get_index)
is using the conda.models.dist.Dist -> conda.models.records.PackageRecord
mapping. Dist class is legacy code that's being phased out, so avoid it.

refs:
- conda/conda-build#5152
- conda/conda-build#5222

Signed-off-by: Marcel Bargull <[email protected]>
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
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants