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

#1893 fix ensuring the consistency when doing signal enrichment #1904

Merged
merged 2 commits into from Feb 22, 2024

Conversation

thjaeckle
Copy link
Member

Resolves: #1893

@thjaeckle thjaeckle added the bug label Feb 16, 2024
@thjaeckle thjaeckle added this to the 3.5.3 milestone Feb 16, 2024
@thjaeckle thjaeckle self-assigned this Feb 16, 2024
@kyberpunk
Copy link
Contributor

Hi @thjaeckle you are very fast, looks perfect. Just one question: how exactly we can enforce the new behavior? Should we somehow setup ditto headers? Thank you.

@thjaeckle
Copy link
Member Author

@kyberpunk I would make this the default behavior - we want to ensure consistency here
There is also an optimisation in place, which does not apply the rather cost-expensive "history lookup" - when there is no newer modification of the thing, the "history lookup" is skipped and the in-memory representation is used for retrieving "extraFields".

That should for 95% of cases optimise and only for high-frequency cases add some additional cost.

@thjaeckle
Copy link
Member Author

@alstanchev @kalinkostashki could you please have a look?
Would want to release a 3.5.3 soon

Could you please run system-tests for this PR as part of a review? That would be super

@kalinkostashki
Copy link
Contributor

@alstanchev @kalinkostashki could you please have a look? Would want to release a 3.5.3 soon

Could you please run system-tests for this PR as part of a review? That would be super

hey @thjaeckle,
sure will do :)

@kalinkostashki
Copy link
Contributor

@thjaeckle, system tests are green
and thank you for re-adding the retrievePartialThing method

Copy link
Contributor

@kalinkostashki kalinkostashki left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@thjaeckle thjaeckle merged commit a5d1b79 into master Feb 22, 2024
3 checks passed
@thjaeckle thjaeckle deleted the bugfix/consistency-signal-enrichment branch February 22, 2024 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Ensure the consistency of signal enrichment for connections
3 participants