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

Job with multiple executions #1964

Merged
merged 37 commits into from
Nov 3, 2023
Merged

Job with multiple executions #1964

merged 37 commits into from
Nov 3, 2023

Conversation

selwin
Copy link
Collaborator

@selwin selwin commented Aug 12, 2023

This PR introduces an Execution object to represent a job's execution. Completion of this PR should allow us to properly track multiple job executions.

@codecov
Copy link

codecov bot commented Aug 13, 2023

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (a044248) 93.61% compared to head (8d6b151) 93.84%.
Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1964      +/-   ##
==========================================
+ Coverage   93.61%   93.84%   +0.22%     
==========================================
  Files          28       29       +1     
  Lines        3757     3897     +140     
==========================================
+ Hits         3517     3657     +140     
  Misses        240      240              
Files Coverage Δ
rq/cli/cli.py 91.60% <100.00%> (ø)
rq/job.py 97.43% <100.00%> (+0.02%) ⬆️
rq/queue.py 95.44% <ø> (ø)
rq/scheduler.py 86.57% <100.00%> (ø)
rq/registry.py 98.00% <92.85%> (-0.39%) ⬇️
rq/executions.py 95.87% <95.87%> (ø)
rq/worker.py 89.39% <89.74%> (+0.88%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@selwin selwin marked this pull request as ready for review October 28, 2023 12:43
@selwin selwin merged commit 3c89f9d into master Nov 3, 2023
27 checks passed
@selwin selwin deleted the executions branch November 3, 2023 10:06
@@ -55,6 +56,13 @@ class JobStatus(str, Enum):
CANCELED = 'canceled'


def parse_job_id(job_or_execution_id: str) -> str:
"""Parse a string and returns job ID. This function supports both job ID and execution composite key."""
if ':' in job_or_execution_id:
Copy link

Choose a reason for hiding this comment

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

This is a problem: when creating a standard job, you can pass a job_id to it, so that instead of using the default uuid, you can use your own custom job id convention.
We use that, and we already use : and | as special separator in the job id. This code change breaks the tacit contract and now a job id can't contain some characters. This could break stuff in production for some users (it will for us...)
Can't the execution id be stored somewhere else ?

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.

None yet

2 participants