Skip to content

Conversation

@ifoughal
Copy link
Contributor

Fixes: #20817

Datasources sync was broken when the user sets a sync interval.

The fix introduces 2 new states:

  • Ready
  • Scheduled

Ready state is set when the data-source object is not new. ie: either updated or changed state recently.
Scheduled is set when the user sets a sync interval.

This PR includes the following logic enhancements:

  • by removing DataSourceStatusChoices.QUEUED from the ready_for_sync method, we let the user sync the data-source anytime even when a daily interval is set, as long as the data-source is enabled.

  • Include the 2 new states (Scheduled, Ready) in the clean & save properties of data-source form model, to set the status correctly depending on the user inputs/

    • If the user creates a new data-source object, then the state is new.
    • if the user update the object with a defined interval, then the states changes to scheduled
    • if the user removed the sync interval, then the state changes to Ready.
    • if a sync is completed either successfully or with a failure, then the status will get update with the job's status as before without impact.

@jnovinger
Copy link
Member

@ifoughal, CI appears to be failing with a test failure.

@ifoughal
Copy link
Contributor Author

@jnovinger

Initially I thought there was an issue with the view rendering class, but after some further TS, and checking the actual value of status on the DB entry, it seems the status field got overwritten somehow.

 id |            created            |         last_updated         | custom_field_data | description | comments | name | type |                           source_url                            | status | enabled | ignore_rules |                   parameters                   | last_synced | sync_interval 
----+-------------------------------+------------------------------+-------------------+-------------+----------+------+------+-----------------------------------------------------------------+--------+---------+--------------+------------------------------------------------+-------------+---------------
 10 | 2025-11-20 10:02:12.763951+01 | 2025-11-20 10:02:19.22684+01 | {}                |             |          | test | git  | https://gitlab.pic.services.prod/orion/IaC/config-templates.git | queued | t       |              | {"branch": "", "password": "", "username": ""} |             |           720

To circumvent this, I run the instance.save() method in the save override of dataSourcesForm. but this generates 2 commits and breaks the test. I'll continue looking into a fix.

@ifoughal
Copy link
Contributor Author

ifoughal commented Nov 20, 2025

@jnovinger

The issue was coming from :

@classmethod
    def enqueue(cls, *args, **kwargs):
        job = super().enqueue(*args, **kwargs)

        # Update the DataSource's synchronization status to queued
        if datasource := job.object:
            datasource.status = DataSourceStatusChoices.QUEUED
            DataSource.objects.filter(pk=datasource.pk).update(status=datasource.status)

        return job

The enqueue method was overriding the state, I have removed datasource.status = DataSourceStatusChoices.QUEUED, the state should then be updated on the clean() method of the post of the datasource form.

@jeremystretch jeremystretch requested review from a team and arthanson and removed request for a team November 20, 2025 14:24
Copy link
Collaborator

@arthanson arthanson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ifoughal Is QUEUED status used anymore? I don't see it ever getting set anywhere.

The QUEUED state was that it was submitted to be run (so pending being run). I think this is should still be a valid status, i.e. get set when it is pending being run (enqueued) and cleared once it is run.

The description for the PR says that two new states are added, but only one is added here, is the description old?

@ifoughal
Copy link
Contributor Author

@ifoughal Is QUEUED status used anymore? I don't see it ever getting set anywhere.

The QUEUED state was that it was submitted to be run (so pending being run). I think this is should still be a valid status, i.e. get set when it is pending being run (enqueued) and cleared once it is run.

The description for the PR says that two new states are added, but only one is added here, is the description old?

I will be reverting the removed of queued as it was indeed needed for when the rq workers as busy. It should then reflect the queued state.

The two new states are READY & SCHEDULED.
Ready is used when the datasource object is not NEW, meaning if the user removes the schedule, so when it changes from interval to None, then the state becomes Ready (ready for sync).
as for scheduled, as its name suggests, is used when the user sets a schedule for the datasource object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Data Sources remove sync interval

3 participants