v2 beta: Django connector seems broken #934
Comments
|
Suggestion: could it be that you're hitting this ? What version of psycopg are you using ? |
|
I'm trying the following script and it works as expected: from __future__ import annotations
import django
django.setup()
from os import environ
import procrastinate
from procrastinate.contrib.django import app
connector = procrastinate.PsycopgConnector(
kwargs={
"host": environ["POSTGRES_HOST"],
"user": environ["POSTGRES_USER"],
"password": environ["POSTGRES_PASSWORD"],
"port": environ["POSTGRES_PORT"],
"dbname": environ["POSTGRES_DB"],
}
)
def main() -> None:
x = app.with_connector(connector)
x.run_worker() # pyright: ignore [reportUnknownMemberType]
if __name__ == "__main__":
main()And it works as expected with the following envvars: (from the dev env, so PG variables are set to match the docker connection defined here. |
|
OK this is great, I can look at this on Monday! I am using 3.1.18. What are you using? This will be a good way for us to sanity check. |
|
I'll triple check bit I'm using the dev env so all my versions are following the poetry lock file. |
|
Yep, 3.1.18 |
|
Ok, very odd. Now when I install It creates a Is this a weird thing on my machine? |
|
And looking at https://github.com/procrastinate-org/procrastinate/releases/tag/2.0.0b1 - it looks like it is behind on main but a bunch of commits - is it force cutting a new beta release maybe with those commits? |
|
And as a sanity check, I re-cloned out repo and resinstalled everything with v1.1.2 and that worked fine. |
|
I’m not on my computer to flesh out a complete answer but I’m pretty sure it’s a |
|
Ya, no rush obviously - 1.1.2 is working great currently! |
|
Ok, the migration issue is solved in #941. Now back to the connection issue. Anything you can share to help investigation is highly appreciated :) |
Sure ! I thought you had mentionned that it was the same to you testing on main and testing on a tag. Either way work for me. https://github.com/procrastinate-org/procrastinate/releases/tag/2.0.0b2 |
|
BTW, instead of: connector = procrastinate.PsycopgConnector(
kwargs={
"host": environ["POSTGRES_HOST"],
"user": environ["POSTGRES_USER"],
"password": environ["POSTGRES_PASSWORD"],
"port": environ["POSTGRES_PORT"],
"dbname": environ["POSTGRES_DB"],
}
)
x = app.with_connector(connector)it might be worth trying: x = app.with_connector(app.connector.get_worker_connector()) |
|
OK, excited to debug this! Dumb question - I don't see a release on pypi for the next version? Also, what is the difference between pypi and "main" of procrastinate. If it is the same I can just have poetry install from a git tag, rather than pypi, but I'd like to be sure :) |
Dammit, the workflow had failed. I
Installing from main is |
|
Awesome, I'll check this out tomorrow now |
|
|
Are you expected to install aiopg to get this to work? I thought we were switching to psycopg3 because it has a good async interface? I'm happy to install aiopg, just wanted to check. |
|
want to point out 2 things:
|
Ah indeed, I could have made it clearer in the docs, and you're right that it might not be the best choice: I've added the psycopg3 compat but I haven't removed Aiopg, however flawed may it be. When determining which one to use, I look at the But then, given that people might have psycopg2 installed but not aiopg, and that I'm going to make a different connection altogether, maybe I should rather:
I'll fix this and make it clearer in the docs. |
|
@hyusetiawan your comment makes me think that you're probably using the last stable release v1.1.2, while this issue is (sorry, unclearly) about the v2 beta. As far as I can tell, the docs are configured to still point to 1.1.2, so maybe you're looking at the docs on the main branch ? That being said, you're more than welcome to try and integrate v2.0.0b2 (it's worth re-reading the Django docs completely) and report any problem here. The v2 had the django integration completely re-done, and in theory it should be much smoother. That said, @paulzakin can attest that so far we're still experiencing road bumps. |
ewjoachim
changed the title
|
ah okay, apologies for the confusion! it's stated in the title even that it's for v2 beta :) |
|
Well... It was not stated on the title at the time of your comment :)
|
|
Ok great @ewjoachim - let me know when you want to me to retest :) |
|
Just tested - no dice :( If you point me in the right direction in the library I can maybe see what logic is not being tripped or not? |
|
It looks like the task exists in the process where you defer, but not in the process where you run the worker. I'm trying to come up with ideas of how that may happen. Can you confirm that:
One thing you can test is go to the def ready(self):
from procrastinate.contrib.django import app
print(app.tasks)On my side, I'm adding some logs, but from your pasted output, it looks like you're not using debug logs, and not formatting them to get the most of them. (structured element). In this PR, I'm providing concrete examples of how you can get all the structured log values without having to add a structured logging library. Output of those logs look like: Which is more informative :) |
|
Ok! Experiment #1. I defined a custom app in settings.py and then imported it throughout the Django app - it all works beautifully. Our application may be "weird enough" that using our app as good test case for the Django integration may be a bad idea... |
|
However, I thought I'd do some more digging to see if I can help you out for other Django people / to satisfy my own curiosity. (PS: I added structured logging, did not really seem to help much) The code below is the crux of the issue. As an experiment, I went "all in" on the Django integration, including running it though the worker. And everything worked great - except for this. Basically, PROCRASTINATE_ON_APP_READY works. The tasks were registered in both the Django and Procrastinate process. So example #2 works great. However, example #2 requires a helper function that imports the app at the time of invocation. If you do not do that, and just run it off the imported app, you get that However, I just want to understand if this behaviour makes sense to you / am I doing things wrong (lol) |
|
Aaaaah yes of course. It will also work if you keep the import outside but do The problem is that when we do Let's see if I can do something about that. Might have to use a proxy instance. |
|
I've added a proxy object, so now, importing the object before it's ready and keeping a reference to it, it will work after the real app is swapped in place. https://github.com/procrastinate-org/procrastinate/releases/tag/2.0.0b7 |
If you had to do weird stuff, everyone had to do weird stuff :) |
|
Ok excited to test this! Also is there a way to add a flag to disable this? For example, we use our Procrastinate/ Django models to insert stuff during tests - I guess switching to the Procrastinate managed connection or models prevents this? Also I feel like I asked about the third Procrastinate / Django model - did that ever get exposed? Procrastinate models exposed in Django, such as ProcrastinateJob are read-only. Please use the procrastinate CLI to interact with jobs. |
|
I guess I can add a setting to control that, and maybe a pytest fixture too. That said, in many cases, I'd say it should be easier to run tests with the InMemoryConnector that is specifically made for testing, exposed and documented. I'd say it depends if we're talking about unit tests or integration test. Btw, I could even expose FactoryBoy factories. So:
|
|
They are integration tests - so having access to the whole thing would be nice! I can make due without them - but it would be helpful |
|
Good news - the proxy works! Other news: Procrastinate models exposed in Django, such as ProcrastinateJob are read-only. Please use the procrastinate CLI to interact with jobs. Only seems to apply to |
Nope, will fix that too. |
|
(I'm removing the DB router from the next beta release. The more I use it, the less I'm convinced it's useful and it will make it harder for some of the changes. If you did set it up, you can remove it.) |
|
Sounds good! |
|
Beta numbers are cheap https://github.com/procrastinate-org/procrastinate/releases/tag/2.0.0b8 The django doc was split into individual pages: https://procrastinate.readthedocs.io/en/main/howto_index.html There are now ways to relax the readonly feature for tests, plus, it's explained how to get a testing connector for the Django app |
|
Ok, more progress and those docs look great. Two things:
Otherwise PROCRASTINATE_READONLY_MODELS solved the problem with tests, so I think with just that healthcheck fix we are good to go! |
Hm,
Within Django:
Because of all that, I didn't include the I could, though, be easily convinced to add it back if you have ideas of things that would make sense checking ? Is your usecase more of a sanity-check for your integration, or a runtime check used as For now I have more questions than answers :D I've fixed |
|
All reasonable points! So if you imagine a Procrastinate & Django application living in a container or pod, and the command is I only ever used it to check whether I could run the worker at all, defering tasks would not be needed for us. Would be good though! I agree though, overall, that the Django connection does obviate some of the healthcheck stuff, as a lot of it is now handled by Django! |
|
Also, side point, is there any way to get on_app_ready to only run once - it seems to run multiple times - once during make migrations, then during migrate, then during app load. That may just be a Django quirk, in which case ignore this :) |
From my understanding of your setup, the issue is that the 3 things you mention are 3 separate processes. The things you do in on_app_ready are in-memory changes (such as registering a blueprint, this is not persisted anywhere) so each process will do its own There may be multiple things you can do:
As a side note, it's perfectly reasonable that your container launches |
|
Yup, that answers my 2nd comment. And yup, the command As for the first comment, the healthcheck, does that answer your question? Otherwise, I think the integration is done, at least from my perspective? |
Ah yes, still pondering on what to do about that (if anything). I think I understand your usecase and it makes sense, but I think it might be better answered than just by activating the I see 2 choices, and we can have both:
|
|
Ya, the second option seems best. This is a really cool integration BTW, I think it will really help with people using the library :) |
|
So, I tried integrating with Django checks framework and... It just didn't match what I wanted to do. Ended up just doing a custom https://github.com/procrastinate-org/procrastinate/releases/tag/2.0.0b9 Hopefully, we're good to go |
|
It works beautifully! Closed, as far as I am concerned :) |
This is about the v2 beta testing.
From discord, from @paulzakin:
The text was updated successfully, but these errors were encountered: