Fix honeypot changes in tasks with cloud data #9010
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #9010 +/- ##
===========================================
+ Coverage 73.40% 73.73% +0.32%
===========================================
Files 418 419 +1
Lines 44296 44362 +66
Branches 3870 3875 +5
===========================================
+ Hits 32516 32709 +193
+ Misses 11780 11653 -127
|
zhiltsov-max
requested review from
Marishka17
and removed request for
SpecLad and
nmanovic
| # Update manifest | ||
| manifest_path = db_data.get_manifest_path() | ||
| if os.path.isfile(manifest_path): | ||
| manifest = ImageManifestManager(manifest_path) | ||
| manifest.reorder([db_frame.path for db_frame in db_frames.values()]) |
There was a problem hiding this comment.
Changing the order of the whole task manifest when updating honeypots for a single job is probably not a good idea.
There was a problem hiding this comment.
Which problem do you see in the current implementation? We still need to pass all the images, because the manifest is a single file.
There was a problem hiding this comment.
Which problem do you see in the current implementation?
I'm not sure whether large tasks will cause problems. This action can probably take more time than 60 seconds. + Perhaps we need to be sure that the other manifest content corresponds to db_frames, otherwise we may not be able to understand why some frames contain unmatched annotations. (I know that such a change will fix problems with unmatching frames from manifest and frame sizes from db, but just logging will be useful, I guess). Up to you.
There was a problem hiding this comment.
I'm not sure whether large tasks will cause problems. This action can probably take more time than 60 seconds.
Why it can take a big amount of time? From what I've seen in the implementation, it should be a file construction and writing, and the file itself is at most several kb in size. Am I missing something?
Perhaps we need to be sure that the other manifest content corresponds to
db_frames
I suppose there are 2 possible reasons for this:
- it's an old task, created before the update and which had some layout updates
- the file was updated in another thread
The first case tasks should just become correct after the next honeypot update.
The second case is described in the endpoint description. There are several places in this function where things can break in such situation. A lock or some other protection mechanism is needed, but we aren't using them for endpoint access currently.
|
Motivation and context
Note: if tasks contained invalid honeypots before the update, they will require an extra manual honeypot update to become correct. There is no automatic fix for this issue.
How has this been tested?
Checklist
developbranchLicense
Feel free to contact the maintainers if that's a concern.