search

maxmind / MaxMind-DB-Reader-python

Python MaxMind DB reader extension
https://maxminddb.readthedocs.org/
Apache License 2.0
176 stars 37 forks source link

Don't require setuptools in prod dependencies in pyproject.toml #155

Closed lewiscollard closed 5 months ago

lewiscollard commented 5 months ago

It's not required for production use, only for building. Instead, add the constraint part to build_system -> requires to save anyone having to ship setuptools in production builds.

Fixes #154.

oschwald commented 5 months ago

Hmm, the Address Sanitizer build failure seems related in some way. I think we would need to figure that out before merging. I reran it and it failed, printing out AddressSanitizer:DEADLYSIGNAL repeatedly. The same build on main works.

lewiscollard commented 5 months ago

Feels like it, if only because the sanitiser job installs setuptools before running the build. I think that might be because older versions of this script were doing python setup.py build to install (where setup.py imports from setuptools). It shouldn't have that same bootstrapping problem with a pyproject.toml-based config.

I'm...going to try removing this just on a hunch. I'll drop the commit if this hunch fails, squash if it works.

lewiscollard commented 5 months ago

Nope, that wasn't it. Actually, it succeeded, but hung forever on exactly the same thing. That exhausts my thoughts for why this might be the case.

lewiscollard commented 5 months ago

Well that's interesting. It succeeded on the most recent run.

oschwald commented 5 months ago

Huh, that is weird. Maybe it is flaky. I don't know that it has a history of being flaky, but it is possible something recently changed in the image to make it so.

oschwald commented 5 months ago

Given the above, I am going to go ahead and merge this. Thank you for taking a closer look.

lewiscollard commented 5 months ago

Thank you for the merge. I was reluctant to suggest merging given that it only started happening with me - but it does rather seem like we just had some bad luck with one particular RAM stick in the world or something.

oschwald commented 5 months ago

Yeah, I am not sure. I don't really see how the issue could be related to your change; I would have expected an explicit build failure if it was that. It did pass again in main.

oschwald commented 5 months ago

I released 2.6.0 with this change.

lewiscollard commented 5 months ago

That was fast. Thank you very much!