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

Propose batch from pre-constructed jobs #1069

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

hkethi002
Copy link
Contributor

@hkethi002 hkethi002 commented Feb 7, 2018

Fixes #940

Changes

  • New Endpoint: POST /batch/jobs with a list of jobs, jobs: [{...}, {...}, ...], in the payload
  • The job maps from the payload are stored in batch.proposal.preconstructed_jobs in mongo
  • Jobs are validated on proposal for gear manifest, inputs, config, and permissions
  • On batch run, the jobs then follow the normal job cycle

Review Checklist

  • Tests were added to cover all code changes
  • Documentation was added / updated
  • Code and tests follow standards in CONTRIBUTING.md

@hkethi002 hkethi002 requested review from kofalt and nagem February 7, 2018 19:17
@codecov-io
Copy link

codecov-io commented Feb 7, 2018

Codecov Report

Merging #1069 into master will increase coverage by 0.04%.
The diff coverage is 97.5%.

@@            Coverage Diff             @@
##           master    #1069      +/-   ##
==========================================
+ Coverage    90.8%   90.85%   +0.04%     
==========================================
  Files          50       50              
  Lines        7029     7063      +34     
==========================================
+ Hits         6383     6417      +34     
  Misses        646      646

@AshleyAncona
Copy link

@nagem and @kofalt this is ready for review

preconstructed_jobs = proposal.get('preconstructed_jobs')

# If Running a batch from already-constructed jobs
if preconstructed_jobs:
Copy link
Contributor

Choose a reason for hiding this comment

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

Break these two branches out into different functions, and call them from here.

@staticmethod
def enqueue_job(job_map, origin, perm_check_uid=None):
def validate_job(job_map, origin, create_job=False, perm_check_uid=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please do not change this function signature. Instead, revert changes to this function, then remove job.insert() to make it consistent with the original documentation. Then check usage of enqueue_job to see who will need job.insert() added after calling.

Copy link
Contributor

@kofalt kofalt left a comment

Choose a reason for hiding this comment

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

@kofalt
Copy link
Contributor

kofalt commented Feb 16, 2018

Notably, this PR is a lot easier to read when using w=1.

@gsfr gsfr removed the request for review from nagem May 11, 2018 14:58
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