Add CI test pipeline and Elasticsearch 7 setup with uv - #291
Conversation
7eacb77 to
49b48ff
Compare
|
@ulgens |
|
Congrats and thank you so much! I can't promise an immediate review but I'll check it bit by bit when I have the time. |
49b48ff to
82d0134
Compare
| matrix: | ||
| django-version: ["4.2.*"] | ||
| python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.14", "3.14t"] | ||
| elastic-version: ["7.17.29"] |
There was a problem hiding this comment.
Do we plan to broaden the elastic entry? How many majors - minors elastic have supported at a given time?
There was a problem hiding this comment.
django-haystack (Primary dependency of this package) doesn't support Elasticsearch 8 or 9 yet.
All earlier Elasticsearch versions are already EOL. Elasticsearch 7 is also EOL, but it only reached EOL about four months ago, so it probably makes sense to keep testing against Elasticsearch 7 for now.
If django-haystack adds support for Elasticsearch 8 or 9 in the future, we can expand the test matrix accordingly.
More info here: https://endoflife.date/elasticsearch
| fail-fast: false | ||
| matrix: | ||
| django-version: ["4.2.*"] | ||
| python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.14", "3.14t"] |
There was a problem hiding this comment.
4.2 doesn't support > Python 3.12 https://docs.djangoproject.com/en/4.2/faq/install/
| { name = "Rolf Håvard Blindheim", email = "rhblind@gmail.com" }, | ||
| { name = "Ülgen Sarıkavak", email = "foss@ulgenwanders.net" }, | ||
| ] | ||
| requires-python = ">=3.8,<3.11" |
There was a problem hiding this comment.
Please don't remove the upper limit. The project doesn't support all existing Python versions and not in a state that it can be installed with a new Python version when it's released.
There was a problem hiding this comment.
I don't get this.
Also, how do we know which Python versions this project supports and which it doesn't, other than relying on the test case results?
The test cases pass for all available Python versions.
According to you, what upper limit can we set? Just < 3.11?
Maybe we can use < 3.15?
Currently, there are 7 client projects in our company using this drf-haystack package. Our 8th and 9th projects use Django 5.2, and because of the Django constraints of this package, we can't use drf-haystack in those projects.
We just copy the whole drf-haystack folder and use it as a normal Django app in the new projects that use Django 5.2 with Python 3.13 and everything working fine.
There was a problem hiding this comment.
Also, how do we know which Python versions this project supports and which it doesn't, other than relying on the test case results?
Yep. Just match the tested versions.
The project may work fine for a particular setup, that's nice, but that doesn't translate to "it works" yet. Please keep the support delegations limited to the tested versions for now.
|
Merged The branch will need a rebase - sorry. |
|
@ulgens As you can see, in the Python 3.8 job matrix, the build backend is causing issues with some license key definition error. I think it's more overhead and extra work that we're currently dealing with without any real benefit. |
|
Dropping 3.8 - 3.9 sounds tempting but if we do that, we will have 0 backward compatibility with the next version, which I think we should fallback to as the very last resort. The CI failure looks like an issue with uv - pyproject and I don't expect it to a require a complex solution. |
82d0134 to
3a51a02
Compare
|
|
||
| BASE_DIR = Path(__file__).parent.resolve() | ||
|
|
||
| SECRET_KEY = "NOBODY expects the Spanish Inquisition!" |
There was a problem hiding this comment.
To keep the diff smaller and easy to review, please don't include things that are not directly in the scope defined by the commit message. Same goes for DEFAULT_AUTO_FIELD and MIDDLEWARE_CLASSES changes. If they are needed for the PR, a separate commit with the explanation would be nice.
There was a problem hiding this comment.
The correct Django settings variable name is MIDDLEWARE, not MIDDLEWARE_CLASSES.
The Django Auto Field setting is used to silence the many warnings related to auto-created primary keys that appear when running runserver or executing test cases.
There was a problem hiding this comment.
Done :)
- Added a new commit with a proper message explaining the middleware-related changes.
- Reverted the secret key and Django Auto Field settings for now. will cover those changes in a separate PR.
| def gdal_is_available(): | ||
| """ | ||
| Return True if GDAL is installed. | ||
|
|
||
| We can't import Point without the GDAL/GEOS libraries, so we just try it | ||
| and treat any failure as "not available". Used to skip the geo tests on | ||
| machines that don't have GDAL. | ||
| """ | ||
| try: | ||
| from django.contrib.gis.geos import Point # noqa: F401 | ||
| except ImproperlyConfigured: | ||
| return False | ||
| return True |
There was a problem hiding this comment.
Did the old helper/utility have any issues?
There was a problem hiding this comment.
The old init code are unnecessary and complex also not optimized along with some Django REST Framework if/else logic and Elasticsearch version extraction.
Also, the message says that the GOES library needs to be installed, but it actually requires GDAL, and GOES is installed automatically as a dependency of GDAL.
…session authentication middleware
3a51a02 to
9d26a37
Compare
|
I think everything is clear now. 🫡 |
…it terminal output

Description
Closes Issues: #249
Closes PR's: #226 and #189
Added support for
Elasticsearch 7in the package.django-haystacksupports up to Elasticsearch 7 version.Other older Elasticsearch versions
<7have reached EOL (End of Life): see.Summary
tox.ini(super slow / unnecessary complexity)uvsupport to manage workflows and project.Dockerfile(We are not using it, docker compose file did the every work)Tests