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

NAS-133708 / 25.04 / Convert job ID to uuid #15454

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

anodos325
Copy link
Contributor

In some CI runs it was observed that unexpected results were being returned for middleware jobs. This commit converts our job ids from being monotonically incrementing integer to proper uuid so that the job id that client is trying to track is guaranteed to uniquely identify it regardless of which HA node is being connected to.

@anodos325 anodos325 added the jira label Jan 21, 2025
@anodos325 anodos325 requested a review from a team January 21, 2025 19:16
@bugclerk
Copy link
Contributor

@bugclerk bugclerk changed the title Convert job ID to uuid NAS-133708 / 25.04 / Convert job ID to uuid Jan 21, 2025
@anodos325 anodos325 force-pushed the convert-job-id-to-uuid branch from b027f7c to 4c7850c Compare January 21, 2025 19:16
In some CI runs it was observed that unexpected results were being
returned for middleware jobs. This commit converts our job ids from
being monotonically incrementing integer to proper uuid so that the job
id that client is trying to track is guaranteed to uniquely identify it
regardless of which HA node is being connected to.
@anodos325 anodos325 force-pushed the convert-job-id-to-uuid branch from 4c7850c to 98e4448 Compare January 21, 2025 22:32
@@ -291,7 +289,7 @@ def __init__(self, middleware, method_name, serviceobj, method, args, options, p
self.app = app
self.audit_callback = audit_callback

self.id = None
self.id = int(uuid4())
Copy link
Contributor

Choose a reason for hiding this comment

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

You can do self.id = uuid4().int

Copy link
Contributor

Choose a reason for hiding this comment

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

@undsoft will such long ints work well with JSON?

>>> uuid4().int
311364041177499672922985169261652976368
>>> uuid4().int
262998890057434762054815435825767399766
>>> uuid4().int
106487518651827065074465048916488123677

Copy link
Contributor Author

@anodos325 anodos325 Jan 22, 2025

Choose a reason for hiding this comment

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

I was hemming and hawing about that one. I may make it a string rather than number (e.g. "106487518651827065074465048916488123677") to make random JSON libraries happier. UI seems to not worry about consuming UUIDs here (nothing broke in an obvious way) as either a number or a string.

@themylogin
Copy link
Contributor

I would fix the client so it forgets all jobs when the connection is re-established.

@anodos325
Copy link
Contributor Author

I would fix the client so it forgets all jobs when the connection is re-established.

It's hard to control how clients will behave (outside of our own).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants