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

Release 22.11.0 #181

Merged
merged 10 commits into from
Dec 2, 2022
Merged

Release 22.11.0 #181

merged 10 commits into from
Dec 2, 2022

Conversation

jezdez
Copy link
Member

@jezdez jezdez commented Nov 30, 2022

Checklist

  • Used a personal fork of the feedstock to propose changes
  • Bumped the build number (if the version is unchanged)
  • Reset the build number to 0 (if the version changed)
  • Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)
  • Ensured the license file is being packaged.

Closes #81

@jezdez
Copy link
Member Author

jezdez commented Nov 30, 2022

Note: Please hold the merge until we have coordinated the release with Anaconda.

@jezdez
Copy link
Member Author

jezdez commented Nov 30, 2022

@jakirkham @mbargull I'd be interested if #81 can be closed given that we're switching to ruaml.yaml?

@jezdez jezdez mentioned this pull request Nov 30, 2022
22 tasks
@jezdez
Copy link
Member Author

jezdez commented Nov 30, 2022

@conda-forge-admin, please rerender

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

@mbargull
Copy link
Member

@jakirkham @mbargull I'd be interested if #81 can be closed given that we're switching to ruaml.yaml?

Probably?
After dinner -- to give an educated answer -- 2022's @mbargull needs to read about what 2019's @mbargull wrote ;).

I love y'all, but time for me to hand it off to more capable people (and quiet my inbox)
@jezdez
Copy link
Member Author

jezdez commented Nov 30, 2022

@mcg1969 Last notification I promise: Thank you for all your work on conda! <3

@mcg1969
Copy link
Contributor

mcg1969 commented Nov 30, 2022

Thank you for everything, team!

@jakirkham
Copy link
Member

@conda-forge-admin, please re-render

@jakirkham
Copy link
Member

I'd be interested if #81 can be closed given that we're switching to ruaml.yaml?

Think so. Added a "Closes #xxx" note in the PR OP, which should take care of that once merged.

That said, would be interested in hearing Marcel's thoughts :)

@@ -41,19 +41,22 @@ requirements:
- conda-package-handling >=1.3.0
- menuinst >=1.4.11,<2 # [win]
- pip
- ruamel_yaml >=0.11.14,<0.17
- ruamel.yaml >=0.11.14,<0.18
Copy link
Member Author

Choose a reason for hiding this comment

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

@beckermr That's something you think should be way more constraint, right?

Copy link
Member

Choose a reason for hiding this comment

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

It depends on your tolerance for risk, but for sure there will be a tradeoff between broken envs, envs that don't solve, and maintenance to update pins.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would at least make the minimum version as big as possible, and I'm not a huge fan of pessimistic upper version pins.

Copy link
Member

Choose a reason for hiding this comment

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

We can patch them out as needed. The issue is breakages which make envs unrecoverable.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is a conda serialize module that redirects to pyyaml, and may or may not always use YAML().something

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'd appreciate a specific proposal of change here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I like the changes here already.

Copy link
Contributor

Choose a reason for hiding this comment

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

conda's serialize module appears to use the current YAML().method form always, instead of the deprecated global yaml.load_safe() or whatever.

@jezdez
Copy link
Member Author

jezdez commented Dec 1, 2022

I'm not sure I understand why it's failing with a missing tqdm import

@jezdez
Copy link
Member Author

jezdez commented Dec 1, 2022

@conda-forge-admin, please rerender

@github-actions
Copy link
Contributor

github-actions bot commented Dec 1, 2022

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/conda-feedstock/actions/runs/3594972530.

@jezdez
Copy link
Member Author

jezdez commented Dec 1, 2022

@beckermr any idea where to look for the pypy related test files run_test.py that are failing?

@beckermr
Copy link
Member

beckermr commented Dec 1, 2022

I don't follow the question @jezdez. If you want to download the failing packages, then use the setting here to have azure store the build artifacts: https://conda-forge.org/docs/maintainer/conda_forge_yml.html#azure

Then you can download them from the azure interface.

@dholth
Copy link
Contributor

dholth commented Dec 1, 2022

Isn't it conda-build creating run_test.py in its environment, which can be preserved with certain conda build flags when running locally? Maybe just to import the imports: tests?

@beckermr
Copy link
Member

beckermr commented Dec 1, 2022

Yep. If you download the built artifacts, you can have conda build just run the tests on them. You'd need a windows machine though.

@jezdez
Copy link
Member Author

jezdez commented Dec 1, 2022

We're skipping PyPy builds for now, to be able to coordinate with the defaults roll out better. More debugging needed on this.

@jezdez
Copy link
Member Author

jezdez commented Dec 1, 2022

@conda-forge-admin, please rerender

@jezdez jezdez merged commit 269508f into conda-forge:main Dec 2, 2022
@jezdez jezdez deleted the release-22.11.0 branch December 2, 2022 09:57
@mbargull
Copy link
Member

mbargull commented Dec 2, 2022

We're skipping PyPy builds for now, to be able to coordinate with the defaults roll out better. More debugging needed on this.

I don't understand this. Didn't you mention defaults would roll out next week?
I'd appreciate it if we were a little more formal when merging builds for infrastructure-critical packages, i.e., require approving reviews (granted, "emergency fix rebuilds" should not still be able to happen fast).

@jezdez
Copy link
Member Author

jezdez commented Dec 2, 2022

Okay, that's good feedback, I think it would be worthwhile to clarify if I should be able to release when things pass or not.

The release on defaults was twice as complicated due to the conda-build, conda-package-handling (which should never have been released during the conda rollout, but was without my agreement), conda-libmamba-solver and conda release bugs on 4 different communication channels. I made the call to release on conda-forge today to have consistency between defaults and conda-forge. I would not have had the energy to wait till next week.

You're suggesting to have a more formal process, I like that. Regarding the people to include in this discussion: The recipe lists a bunch of people as maintainers of this feedstock, but I wonder if they are all active? What is the appropriate best practice to make sure only people actively working on a feedstock are listed there?

@mbargull mbargull mentioned this pull request Dec 2, 2022
5 tasks
@mbargull
Copy link
Member

mbargull commented Dec 2, 2022

Okay, that's good feedback, I think it would be worthwhile to clarify if I should be able to release when things pass or not.

Yes, I just want us to be careful here because we easily break people's daily work if something's wrong with our package manager's own package ;).
Unfortunately, I haven't look into conda's own source code lately so could gauge how big the changes of this release are. With the new plugin manager and all active plus unvendoring it just sounded like a big one to me, so I was surprised to see this merged so quickly. (But I'm also known to worry too much... ;)

The release on defaults was twice as complicated [...]

Sorry to hear about the trouble :/

The recipe lists a bunch of people as maintainers of this feedstock, but I wonder if they are all active?

A bunch aren't active (overall in conda-forge or just in maintaining this feedstock).
So, I don't think we have to be uber-formal in requiring everyone's/many reviews, but maybe at least 2 or so.

What is the appropriate best practice to make sure only people actively working on a feedstock are listed there?

Good question ;). I don't think we have a best practice. If we wanted to weed out the list, we could open an issue asking who wants to continue to maintain here with a checkbox list and keep that open for X months and then remove the unresponsive and ones that don't have interest/time for maintenance. (We can always add them back.)

@mbargull
Copy link
Member

mbargull commented Dec 2, 2022

[...] we could open an issue asking who wants to continue to maintain here with a checkbox list [...]

Ha, and you already did ❤️ #188

@jezdez
Copy link
Member Author

jezdez commented Dec 2, 2022

Okay, that's good feedback, I think it would be worthwhile to clarify if I should be able to release when things pass or not.

Yes, I just want us to be careful here because we easily break people's daily work if something's wrong with our package manager's own package ;). Unfortunately, I haven't look into conda's own source code lately so could gauge how big the changes of this release are. With the new plugin manager and all active plus unvendoring it just sounded like a big one to me, so I was surprised to see this merged so quickly. (But I'm also known to worry too much... ;)

That's fair and frankly something I've worried a little as well, mostly about my own bias after working on this stuff so long, I lack the bird's-eye view sometimes. Thank you for calling this out!

The release on defaults was twice as complicated [...]

Sorry to hear about the trouble :/

Nobody's fault, no need for an apology :)

The recipe lists a bunch of people as maintainers of this feedstock, but I wonder if they are all active?

A bunch aren't active (overall in conda-forge or just in maintaining this feedstock). So, I don't think we have to be uber-formal in requiring everyone's/many reviews, but maybe at least 2 or so.

Good idea, I'm all for making it explicit what to expect and like GitHub's required reviews. We've been using it for conda and a few other repos successfully to force consensus making.

What is the appropriate best practice to make sure only people actively working on a feedstock are listed there?

Good question ;). I don't think we have a best practice. If we wanted to weed out the list, we could open an issue asking who wants to continue to maintain here with a checkbox list and keep that open for X months and then remove the unresponsive and ones that don't have interest/time for maintenance. (We can always add them back.)

Super good! <3

@mbargull
Copy link
Member

mbargull commented Dec 2, 2022

[ so many agreeable words ]

@jezdez, interacting with you is always so constructive and pleasant! Suspiciously good, that is... 😆

@jakirkham
Copy link
Member

The other thing worth noting (in addition to what Marcel has nicely laid out) is it can be helpful to roll out packages at the beginning of the week when we know (and have confirmed) a few people are around and ready to handle issues that may crop up. For the most part we have done pretty good at doing this (for example moving releases after holidays as opposed to before). Sorry don't mean to be piling on, but wanted to add this small piece of feedback.

@mbargull
Copy link
Member

mbargull commented Dec 7, 2022

@jakirkham @mbargull I'd be interested if #81 can be closed given that we're switching to ruaml.yaml?

Probably?
After dinner -- to give an educated answer -- 2022's @mbargull needs to read about what 2019's @mbargull wrote ;).

Some dinners ago I've re-educated myself but actually didn't answer yet...

We probably could/should address conda-forge/ruamel.yaml-feedstock#119 at some point.
But we are now also adding test/downstreams to ruamel.yaml/ruamel.yaml.clib so we can catch some incompatibilities with conda when those packages are updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants