Skip to content

Maintenance: Update to mypy v1.13.0 #654

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

Merged
merged 1 commit into from
Feb 14, 2025
Merged

Maintenance: Update to mypy v1.13.0 #654

merged 1 commit into from
Feb 14, 2025

Conversation

amotl
Copy link
Member

@amotl amotl commented Sep 20, 2024

Problem

Trying to update to Python 3.11 makes mypy croak.

fatal error: code.h: No such file or directory

-- https://github.com/crate/crate-operator/actions/runs/10957798573/job/30426680335?pr=549#step:6:91

Solution

Let's update mypy first.

@amotl
Copy link
Member Author

amotl commented Sep 20, 2024

Hi. This patch needs a few adjustments to satisfy the type checker. Resolving them probably needs better insights into the code base than what I can provide.

/cc @goat-ssh, @Taliik, @tomach

https://github.com/crate/crate-operator/actions/runs/10958103493/job/30427634720?pr=654

@amotl amotl force-pushed the collab/update-mypy branch from ba76c5c to 2fbdc79 Compare November 5, 2024 17:04
@amotl amotl changed the title Maintenance: Update to mypy 1.11.2 Maintenance: Update mypy Nov 5, 2024
@amotl amotl force-pushed the collab/update-mypy branch 4 times, most recently from d5ae08b to dfbe1ca Compare November 5, 2024 18:29
@amotl amotl requested review from Taliik and tomach November 5, 2024 18:29
@amotl amotl changed the title Maintenance: Update mypy Maintenance: Update to mypy v1.13.0 Nov 5, 2024
Copy link
Member Author

@amotl amotl left a comment

Choose a reason for hiding this comment

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

Hi. I've just updated this patch, and CI signals success, so I am handing it in for review. On a few spots it is obvious it needs more love. On others, I am not sure the right adjustment has been applied. On others again, I believe the updates are just right.

In other words, you need to diligently review and eventually take over this patch, to make it converge well. It aimed to make a start on this topic at least, in order to apprach upgrading to support more recent versions of Python.

@amotl amotl marked this pull request as ready for review November 5, 2024 18:34
@amotl amotl force-pushed the collab/update-mypy branch from dfbe1ca to 7beaad8 Compare November 6, 2024 10:58
@amotl amotl force-pushed the collab/update-mypy branch from 7beaad8 to 59316c6 Compare December 13, 2024 01:20
@tomach tomach mentioned this pull request Dec 16, 2024
5 tasks
@amotl amotl force-pushed the collab/update-mypy branch 2 times, most recently from af30858 to 5d0ae35 Compare December 16, 2024 19:15
@amotl amotl force-pushed the collab/update-mypy branch from 5d0ae35 to 0f34067 Compare February 11, 2025 14:18
@amotl
Copy link
Member Author

amotl commented Feb 12, 2025

Hi there. Resolving this would unlock a whole bunch of patches stacked upon this one. However, this particular patch needs a few dedicated cycles by authors more accustomed with the code base.

Maybe you can afford to spend a few minutes on this, @tomach? 🙏

@tomach
Copy link
Contributor

tomach commented Feb 13, 2025

Hi @amotl! Thanks for the reminder. I’ll be taking a look at this today. Thanks for your effort and patience! 🙏

@tomach
Copy link
Contributor

tomach commented Feb 13, 2025

I tried to fix all mypy errors in crate/operator/prometheus.py and tests/test_metrics.py. And changed one fix in crate/operator/handlers/handle_create_cratedb.py where I think raising an error if status.get(CLUSTER_CREATE_ID) is undefined could cause issues, because it might be the case in the first run of a create-operation (but not 100% sure about that).

There is still one issue with sphinx now... 🙈

@amotl
Copy link
Member Author

amotl commented Feb 13, 2025

Thank you very much. I am sure you can also tame the Sphinx, otherwise please ping me about it anytime 1.

Footnotes

  1. Single possible winter day with snow outside in Berlin, so I am going sledging with the dwarf now, and will join later again.

@amotl
Copy link
Member Author

amotl commented Feb 13, 2025

You made it, thanks a stack! 💯

Unfortunately, I can't approve the PR because I created it, so we need to ask @juanpardo, @Taliik, or @plaharanne, or fall back to @surister or @kneth. Feel free to squash commits and merge at your disposal.

@tomach
Copy link
Contributor

tomach commented Feb 13, 2025

Yes, thank you! I added it to the nitpick_ignore list. Since it is just an interface and there is no official docs for prometheus_client on readthedocs, this should be fine I think.

@amotl
Copy link
Member Author

amotl commented Feb 13, 2025

I think it's fine. The same thing happens in a future patch about another library that doesn't employ docs yet.

Copy link
Contributor

@plaharanne plaharanne left a comment

Choose a reason for hiding this comment

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

👍

This patch intends to contribute to unlock upgrading to Python 3.11.
@amotl amotl force-pushed the collab/update-mypy branch from 737a1b8 to 2e687fc Compare February 14, 2025 09:17
@amotl amotl merged commit fe823c5 into master Feb 14, 2025
5 checks passed
@amotl amotl deleted the collab/update-mypy branch February 14, 2025 10:15
@tomach tomach mentioned this pull request Feb 14, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants