Used tracdb instead of Trac RPC endpoint in dashboard #1479
Conversation
bmispelon
force-pushed
the
dashboard-tracdb-instead-of-api
branch
5 times, most recently
from
b03c112 to
310a133
Compare
|
Hey @bmispelon , I have started to look at it, looks good to me, I have to reinstall the project (don't ask me why) and I will review it fully then. |
|
Hi Sarah and thanks for taking a look! This PR is independent from the one about upgrading Django and could absolutely be merged before. In fact, I'd like it to be deployed so I can remove some dependencies from code.djangoproject.com. |
|
@sabderemane have you had a chance to look at this? Is there something I can do to make the review easier? |
There was a problem hiding this comment.
@bmispelon Thanks 👍 Looks good 🚀 it's worth trying 🤞
Are we going to remove xmlrpcplugin from code.djangoproject.com?
|
Thanks for the review! ✨
Eventually I would like to, yes (I don't trust that it's being maintained and I find it hard to audit what capabilities it actually gives, and how those capabilities interact with Trac's permissions). |
bmispelon
force-pushed
the
dashboard-tracdb-instead-of-api
branch
from
6360563 to
30b196b
Compare
| TicketCustom.objects.bulk_create( | ||
| [ | ||
| TicketCustom(ticket=ticket, name=name, value=value) | ||
| for name, value in custom.items() | ||
| ] | ||
| ) |
There was a problem hiding this comment.
@bmispelon I'm coming from #1792 (comment)
Model.objects.create(...) correctly creates the object in the db
This is not working with 5.1 🙁 I was trying to debug it, but I couldn't understand how did this work in the first place. We have TicketCustom.ticket.primary_key = True in place, and the docs say
primary_key=True implies null=False and unique=True.
So we shouldn't be able to create multiple TicketCustoms with the same ticket value in the first place (I guess) but somehow it was working fine.
Do you have any ideas about why would this work fine until now but start breaking with Django 5.1?
Hi,
For some context, I've been working on code.djangoproject.com recently and I would like to propose removing the API page at code.djangoproject.com/rpc. However it was pointed out to me that dashboard.djangoproject.com actually uses that API to gather some of its statistics.
So my initial goal was to rewrite the dashboard metrics to make use of the
tracdbmodels instead.I wanted to have a few tests to make sure I wasn't breaking everything, which is why this PR got slightly out of hand. It turned out that testing those unmanaged
tracdbmodels was more of a challenge than I'd anticipated but I think I landed on something somewhat readable (and hopefully not too horrible) in the end.A nice side-effect is that this PR removes the need for two of the database views (which are a maintenance burden and caused us some downtime a few weeks ago after an upgrade).