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

Add job document comparator to ignore pre-signed difference in s3 url #473

Merged
merged 11 commits into from
Nov 14, 2024

Conversation

ig15
Copy link
Contributor

@ig15 ig15 commented Nov 11, 2024

Motivation

  • Please give a brief description for the background of this change.
    Currently 2 job documents containing the same s3 pre-signed url that are sent from the console are compared directly in string formatting. After these changes the 2 job documents will be compared neglecting the aws temporary access signature, to avoid 2 identical job documents with different signatures being considered different and creating conflicts on the device processes

Modifications

Change summary

Please describe what changes are included in this pull request.
Added a comparator function to replace the pre-signed part of an s3 pre-signed url with a common placeholder to avoid
same job with different pre-signed parts to be inferred differently by device client

Revision diff summary

If there is more than one revision, please explain what has been changed since the last revision.

Testing

Is your change tested? If not, please justify the reason.
Please list your testing steps and test results.

Changes tested locally by simulating different cases for the job documents.
Manually tested the output against customer job document and handler files as well.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

source/jobs/JobsFeature.cpp Outdated Show resolved Hide resolved
source/jobs/JobsFeature.cpp Outdated Show resolved Hide resolved
source/jobs/JobsFeature.h Outdated Show resolved Hide resolved
source/jobs/JobsFeature.h Outdated Show resolved Hide resolved
test/jobs/TestJobsFeature.cpp Outdated Show resolved Hide resolved
source/jobs/JobsFeature.cpp Show resolved Hide resolved
@ig15 ig15 self-assigned this Nov 12, 2024
@ig15 ig15 marked this pull request as ready for review November 12, 2024 09:36
Copy link
Contributor

@RogerZhongAWS RogerZhongAWS left a comment

Choose a reason for hiding this comment

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

LGTM

source/jobs/JobsFeature.cpp Show resolved Hide resolved
@ig15 ig15 merged commit ce63afe into awslabs:main Nov 14, 2024
14 of 41 checks passed
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.

4 participants