Skip to content
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

[14.0] connector_search_engine: keep data for bindins to check #192

Open
wants to merge 2 commits into
base: 14.0
Choose a base branch
from

Conversation

simahawk
Copy link
Contributor

@simahawk simahawk commented May 9, 2024

To help checking what the problem might be
do not skip storing the computed data.

@simahawk simahawk force-pushed the 14-fix-to-be-checked branch from fb3e69d to 6302d6f Compare May 9, 2024 13:31
simahawk added 2 commits May 9, 2024 15:37
To help checking what the problem might be
do not skip storing the computed data.
Before this change the error was visible only in the job
if not deleted.
This we can provide more info to the end user
w/o the need for a new field.
This info will be wiped on the next successful compute.
@simahawk simahawk force-pushed the 14-fix-to-be-checked branch from 6302d6f to 0a7bf69 Compare May 9, 2024 13:38
vals = {"data": index_record}
if binding.sync_state != "to_update":
vals["sync_state"] = "to_update"
vals["date_modified"] = fields.Datetime.now()
if error:
vals["sync_state"] = "to_be_checked"
vals["data"]["__error__"] = error
Copy link
Contributor Author

@simahawk simahawk May 9, 2024

Choose a reason for hiding this comment

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

Of course it would be better to have a specific field. However:

  • we could have tons of records on different models
  • in a pre-migrate step you don't have at hand all the models that could inherit from the mixin as they are not loaded yet
  • we cannot safely add the field via sql to all the tables that would need it

Or, at least, I don't have a smart way to do it...
This is better than nothing for now :)

Any recommendation?

Copy link

github-actions bot commented Sep 8, 2024

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Sep 8, 2024
Copy link
Member

@mmequignon mmequignon left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants