task sync manager preparations #835
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
osoukup
force-pushed
the
task-sync-manager
branch
2 times, most recently
from
15a2e8d to
d30cf38
Compare
osoukup
added
the
technical
osoukup
force-pushed
the
task-sync-manager
branch
2 times, most recently
from
ce1ffca to
8c6bf9f
Compare
osoukup
force-pushed
the
task-sync-manager
branch
from
8c75fc5 to
9360a0e
Compare
|
I am not getting any reviews and have to continue working so this PR is growing and growing... Sorry to the future reviewers that it will probably going to take some time but going commit-wise should help understand. |
jobselko
reviewed
Dec 5, 2024
as part of the complete fetch-back on sync removal and as it is done by the following task collector run anyway
as part of the complete fetch-back on sync removal and as to save one more unnecessary Jira API query
as most of the time no return values are needed
it used to make sense as we used to save the flaw to the DB at the end of the save machinery but we reversed the order so now there should always be an existing flaw or an error
while leaving the timestamp just being crafted on our side before the actual request is being made so it is at or before the actual Jira timestamp not preventing the task sync-back by the collector which will eventually correct the timestamp in case it is in a bit of desync this may result in a second or a few seconds desync for a short while on the task update timestamp which is however acceptable and will save us some Jira calls syncing the task back synchronously which happens async by the collector later anyway
as it happens after save to DB
to shift the decission responsibility from the serializer to the model and to disallow forcing model to update when there is no actual change which would result in an extra Jira query without no effect as Jira ignores such queries
so they can be later called by either the synchronous procedure or by the async sync manager
for now the creation is out of scope as I am writing this as part of task management rework to async and I need to address the diff param no being provided on update the way the serializers used to work but actually this is overall very important going forward as so far the serializers were bypassing the save method through the default update resulting in call without parameters and then calling it again potentially multiple times providing the parameters or directly calling task synchronization etc. which was bad
which is already inherited from other mixin
to start performing the updates the correct way for now I do not specifically care for affects and trackers and other models as this is already getting out of scope of my task but eventually we should update everything properly
this is a bit unfortunate but the PackageVersion related serializers are so different from the rest that they do not work with the new BaseSerializer without making it pretty ugly which would be a step backwards and I do no want to extend the task scope to rewriting the PackageVersion stuff so I am basically leaving it as it was before
osoukup
force-pushed
the
task-sync-manager
branch
from
9360a0e to
a3a8689
Compare
jobselko
approved these changes
Dec 9, 2024
JakubFrejlach
approved these changes
Dec 10, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a move towards async task management. It aims to simplify the stuff a bit and remove some more unnecessary Jira API calls. This time we focus on the calls which sync back the task right after we updated it. We will just assume that when the update is successful the Jira issue will have the values we set. We also set the task last update timestamp on our side which may be a little bit before the actual update in Jira but will be synced by the collector in a short while so no big deal. A number of test changes were necessary along the way.
I added some more parts. I reworked the decision logic regarding the task management and moved some functionality so the execution parts are at the end separated methods which can be called both synchronously and async.
Then I run into an issue that the broken way serializers update the flaws (and other and create but that was put aside) means that the save and all the parameters are not used all at once but it is called multiple times with different context - or tasksync used to be called completely separately outside of the
savecall. It required quite some serializer rework which is definitely not complete but already brought what I needed. I think that the result is actually simpler and more understandable then it was before but I am glad for any feedback.