-
-
Notifications
You must be signed in to change notification settings - Fork 157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
4773 Allow iquery probes to trigger sweep tasks for existing Dockets #4953
4773 Allow iquery probes to trigger sweep tasks for existing Dockets #4953
Conversation
… if the docket already exists Fixes: #4737
…cketFactory creation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, thank you. Looks about right to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
note: I'm holding off on merging this PR until @albertisfu provides instructions for enabling the daemon and we create separate issue to prioritize those tasks. |
Thanks @ERosendo for the review! @mlissner I think the PR can be safely merged since I remember you disabled the daemon. However, if you want to confirm, the setting should be Before getting the daemon running again, since in #4773 you mentioned that we should start the sweep from 2020, we should run the following command:
We can confirm the values set with this script:
After this is set, we should confirm that the |
I confirmed the setting is correct. It was disabled, but the variable name was wrong, so I fixed that. We have an issue for launching the probe here: https://github.com/freelawproject/infrastructure/issues/208 I just set it so that it's assigned to me, Alberto, and Diego. @chaco-fl, if you can take point on this one when you're ready, that would be great. I think Alberto should be able to help you understand anything you need, and I'm happy to help too, since I set up the daemon. Maybe a three-way call would be a good way to do this, when you're ready, Diego. :) |
This PR resolves the issue described in #4773, where the last probe in a probe cycle encountered an existing docket, preventing the sweep task from being executed. Previously, one of the conditions for triggering a sweep was that the docket had to be newly created. While this condition is useful for triggering sweep tasks once we catch up with court content and enable
IQUERY_SWEEP_UPLOADS_SIGNAL_ENABLED
, we currently need the probe daemon to trigger sweep tasks even for existing dockets in the last probe.To address this, I updated the conditions for triggering
update_latest_case_id_and_schedule_iquery_sweep
in thehandle_update_latest_case_id_and_schedule_iquery_sweep
signal.Additionally, after tweaking the logic to allow sweep tasks for existing dockets, some related tests began failing specifically, tests ensuring the sweep task was called the expected number of times. I found the issue was caused by
DocketFactory
, which was triggering thesave
method twice: once to save the instance for the first time and again within thepost_generation
hook.After discussing this with Eduardo, he explained to me that the hook is necessary to avoid calling S3 when generating
DocketFactory
instances with thefilepath_local
field so removing the hook was not an option. To resolve this, I replaced thesave
method in the hook with aqueryset.update
call, which persists thefilepath_local
field without triggering signals.