django
Do we really need tox?
While working with https://github.com/django/djangoproject.com/pull/1792, I realized that tox setup has some issues, and I tried to understand what is going on. Apparently, there is only one version of Python and Django that we test againist, and the tox config doesn't look like it does more than chaining tests and linting. tox is not needed for Django tests and the rest can be handled by pre-commit.
I think it'd be great if we could remove it.
I wonder if tox is useful for the pattern seen in these two commits where a new Python version is added to test against along side the old one before the old one is removed:
- 164c84027f725015697dbf5f7e6f08ae5dd9dcf7
- 70895c194e5e300e8130a6353a8af395f8177786
I'm looking forward to hear what others think!
It should be. I personally think that would be ideal, but I don't know the current process.
I also don't think we need tox here. Maybe we can open a draft PR to see all the required changes to get rid of it.
I think upgrading without tox will be fine, but it'd be nice to leave in place the other mechanisms for supporting 2+ Python versions at the same time. I left a comment on the PR: https://github.com/django/djangoproject.com/pull/2135#pullrequestreview-3058066493
@tobiasmcnulty The discussion under https://github.com/django/djangoproject.com/pull/2135#discussion_r2232902387 was getting a bit too long for a review comment, let's continue here 🌻
There are several discussion points, so I'll share my thoughts and answers in sevaral parts. Please let me know if I'm mistaken or missing anything.
The website is currently running with Python 3.12. ~As far as I understand, we don't have a configuration pointing to a production env's Python version in the repository, which is something we need to improve~ *. However, both the README and the Dockerfile refer to 3.12.
* This was a misunderstanding on my end.
https://github.com/django/djangoproject.com/pull/1640
The PR adding support for 3.12 claims that it is "optional support". I was looking to understand what this means, and I saw
I deployed this to both the production (Python 3.8) and preview (Python 3.12).
That PR updates the Dockerfile to 3.12, but the comment says production is still using 3.8. I suppose there were reasons to do this at the time, but using different versions in production versus development is an anti-pattern, which I also believe is the root of the complexity and confusion surrounding Python version support in this repository. For the sake of simplicity, if we are deploying from the main branch, we shouldn't merge a new version support to the main branch until we are ready to deploy. With the newly established Website Working Group, we have sufficient resources to handle these types of updates in a healthier manner and without requiring workarounds, such as the aforementioned optional support.
I was a bit confused about Trac, sorry for that. The source of my confusion is https://github.com/django/code.djangoproject.com/pull/219, which is in a different repository. I guess we somehow would like to use similar Python versions in both projects, but I don't think there is any reason to support 3.11 in djangoproject.com for the sake of Trac integration.
In the end, my goal with these PRs is to simplify the configuration and the development experience and removing the "optional support" approach is a step towards that goal. Because this project is not a library or framework - it's a website project that we deploy into an environment we control, with the correct approach, we won't need to support multiple Python versions simultaneously. If we ever want to update to Python 3.13, my approach would be to create a new branch, do the development and testing on that new branch, deploy it to a preview/staging environment if necessary, and then merge to main when we are satisfied with the changes.
tl;dr: If you and the Website Working Group are committed to testing the branch thoroughly in a preview/staging environment--and perhaps if we make smaller upgrades more frequently 😅--I support the branch approach.
(I should clarify: I defer to @bmispelon on how to manage the branches between production/preview--if anything I've said conflicts with those recommendations, please disregard.)
Hi @ulgens,
That PR updates the Dockerfile to 3.12, but the comment says production is still using 3.8.
This is correct. At the time, we were also migrating from an older virtual environment-based deployment. Consequently, production remained on Python 3.8 until the team was satisfied with the upgrade to 3.12/Docker.
Now that both Trac and the website have transitioned to Docker, we are no longer constrained by the operating system for the Python version, and the Dockerfile serves as the definitive reference for the deployed environment.
On a different project, I've previously built two distinct Docker images--one for each Python version--to facilitate a similar upgrade scenario.
using different versions in production versus development is an anti-pattern
I agree, and I actually view this as an argument in favor of supporting both versions simultaneously during an upgrade. There's no shortcut to avoiding the need for the team to assess and consider the implications of both versions for a period (both locally and in deployed environments). By embedding the mentality that we need to support both versions for a transitional period, we reduce the likelihood of deploying a backward-incompatible upgrade.
we shouldn't merge a new version support in until we are ready to deploy
my approach would be to create a new branch, do the development and testing on that new branch, deploy it to a preview/staging environment if necessary, and then merge to main when we are satisfied with the changes
In my view, this approach (a) risks underestimating the need to test the upgrade in a deployed environment for a sustained period and (b) makes rolling back more challenging if things don't go as planned.
Of course, it is possible to conduct all the same testing regardless of whether the upgrade is maintained in a separate branch, and to be reasonably confident that things won't break if the branch was deployed and tested in a preview/staging environment. Perhaps I'm being overly cautious because 4 major versions of Python at once was not a trivial change and deserved more care and diligence than a smaller upgrade.
All that said, I'm okay with the branch approach, provided it is tested thoroughly in a staging/preview environment before we attempt to deploy it to production (in particular, its integrations with external systems such as Trac and Stripe), and more so if we undertake smaller and less risky upgrades.
provided it is tested thoroughly in a staging/preview environment before we attempt to deploy it to production (in particular, its integrations with external systems such as Trac and Stripe), and more so if we undertake smaller and less risky upgrades.
This is the goal 🙂 Having a less complex and less fragile development environment, with more reliable testing. Once we have a working Docker setup, my next step is to identify opportunities for improvement in testing and proceed accordingly.
Hey everyone, thanks for your work on this!
Before I got too much into the weeds, I wanted to say that I'm in favor of removing tox.
It seems to me that the last remaining question is whether we want to have support for testing multiple versions of Python. I don't have a particularly strong opinion on that one. It seems to me that in the day-to-day work this is not a feature we need, and it only proves useful in times of transitions/upgrades. In that sense I'd be in favor of removing the multiversion support. But on the other hand having a matrix with a single version and a comment as Tobias suggest doesn't seem like a big overhead, and if it can make a future upgrade smoother then it's a good argument for keeping it.
With all that said, I also wanted to provide a bit of context to the dependency between Trac (aka code.djangoproject.com) and the website (this repo). This is where the rambling will start, be warned 😅
Rambling (click to unfold)
With the two projects now running in separate containers, we can now easily use different Python versions for both, and we're also not tied to the host system's Python version.
But there is an invisible dependency between the two projects that does sort-of constrains how big the version-gap can be between the two repositories: they share a database.
Indeed, our Trac instance supports logging in with your djangoproject.com credentials. This is achieved in a slight Frankenstein way by running Django inside Trac so we can use Django's contrib.auth forms and models.
While Django is very good at backwards compatibility, I don't think this guarantee extends to the database schema. What I mean is that it's not a supported use-case to be having two projects with different Django versions talking to the same database (I could be wrong on this, happy to be linked to documentation that would say otherwise).
It's not a very high risk, especially considering how limited our usage of Django is inside Trac, but it exists and for that reason I've in the past tried to keep the Django versions the same on both djangoproject.com and code.djangoproject.com (though evidently I've not been very consistent at it since Trac currently runs Django 5.1, oops 😅 ).
So anyway, all that is to say that there is a tiny dependency between the two projects. That dependency probably doesn't matter in practice and will probably not be a problem, but I wanted to mention it because I think it adds context and can explain some of the technical decisions that have been made historically. Thanks for reading my ramblings until the end 😁
@bmispelon Thanks for your comment and all the info.
To, hopefully, finalize this discussion, let me summarize what we can do here:
- Remove tox. It seems we all agree on this. I can remove the matrix change from the PR.
- If that's okay, I can proceed with 3.13 update and demonstrate why we don't need the matrix anymore.
Following the pattern 😄 , my ramblings can be found below
The second part comes with some more questions and thoughts:
-
Question: Can we update to 3.13? I have mixed feelings about this. If we are going to continue with Trac (which I believe we shouldn't), we need to decide whether to stay with the latest Python version supported by Trac or upgrade it and address any potential issues that may arise. If we are not planning to update to 3.13 anytime soon, keeping the remedies of an old update and the discussion of whether to keep it or not loses its meaning.
-
Thought: I do understand that this "foolproof" way of updating the Python version was the easier way at the time, especially considering the update covered 4 major versions, but I also believe that it wasn't the best. In yesterday's meeting, @bmispelon 's answer for one of my questions about the deployment process was (sorry if I can't match your exact words) "We are trying to achieve the easier solution first". I understand this approach, and I support it, but I feel like we are missing that there should be follow up steps: First do it, then do it right, then do it better We have the resources to do things right and better, and saying that (summarizing what I hear from the opposition to remove the matrix) (I know these are not the exact words but if you remove all the niceties, this is what is left) "No you shouldn't try that, we did this way in the past and we will continue doing it like that" doesn't contribute to the improvement of the overall quality here. We may not have enough resources to take risks like changing the whole deployment process at once, but we have ways and resources to improve testing if we lack confidence in the project's tests. If we need automated preview deployments, we can develop the workflow. If we need a quick way to revert a deployment, we can develop that. We need to name the issues instead of workarounds so that things can be improved.
Having all these multi-week discussions over a line of configuration that does not even affect us right now feels weird. Still, it allowed us to do a reality check and understand the development approaches of different parties. I hope it was beneficial for everyone.
Remove tox. It seems we all agree on this. I can remove the matrix change from the PR.
Yes, we all agree that tox can go. It's been good to us in the past, but it doesn't spark joy anymore 😂
If that's okay, I can proceed with 3.13 update and demonstrate why we don't need the matrix anymore.
Absolutely. It should be a separate PR than removing tox though, the two things might be connected but we should still do them one at a time.
[...] but I also believe that it wasn't the best [...]
That's the benefit of hindsight. The way I look at it, the reason we're able to have this discussion today is because we have a website that's been running continuously for 20 years. Best be damned, that's a world-class achievement in my eyes. And that's thanks to people like Tobias who do put in their best.
Having all these multi-week discussions over a line of configuration that does not even affect us right now feels weird.
I can understand the frustration, but considering we're all volunteers doing this in our free time, I think a couple of weeks is not that bad (especially in the middle of summer holidays for a lot of us). I may be biased, but I think it's fair for the ops team to be pushing for conservatism here since we're the ones who are ultimately responsible for deploying the changes (I believe that's part of the charter for the website working group).