Skip to content

Fix consistency for job id generation within codebase.#934

Merged
mariacarmina merged 13 commits into
mainfrom
update-job-id-generation
May 29, 2025
Merged

Fix consistency for job id generation within codebase.#934
mariacarmina merged 13 commits into
mainfrom
update-job-id-generation

Conversation

@mariacarmina
Copy link
Copy Markdown
Contributor

@mariacarmina mariacarmina commented May 1, 2025

Fixes #935 .

Changes proposed in this PR:

  • make generateUniqueId util function for generating job ids
  • startCompute can receive jobId as optional parameter, because paid start compute handler generates before a job id that can be used and not re-generated, in case this parameter jobId is not provided, docker compute engine will generate a new one based on generateUniqueId output
  • update parameters for unit tests when inserting new compute job

@mariacarmina mariacarmina self-assigned this May 1, 2025
@mariacarmina mariacarmina marked this pull request as ready for review May 1, 2025 13:07
Copy link
Copy Markdown
Contributor

@giurgiur99 giurgiur99 left a comment

Choose a reason for hiding this comment

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

Don't we need to add the jobId in FreeComputeStartHandler too? And we can make it non-optional.

@mariacarmina
Copy link
Copy Markdown
Contributor Author

Don't we need to add the jobId in FreeComputeStartHandler too? And we can make it non-optional.

We can generate the jobId within free start compute handler and remove the generation from engine.startComputeJob

Copy link
Copy Markdown
Contributor

@giurgiur99 giurgiur99 left a comment

Choose a reason for hiding this comment

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

LGTM!

@mariacarmina mariacarmina merged commit c9faa2f into main May 29, 2025
21 of 22 checks passed
@mariacarmina mariacarmina deleted the update-job-id-generation branch May 29, 2025 13:40
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.

Fix consistency for job id creation

3 participants