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 batch_job_id to result metadata #295

Merged
merged 5 commits into from
Nov 12, 2024

Conversation

rugeli
Copy link
Collaborator

@rugeli rugeli commented Oct 28, 2024

Problem

What is the problem this work solves, including
closes #296

Solution

What I/we did to solve this problem

  • batch job ids are stored as environment tokens, we can retrieve them via os
  • add a batch_job_id field to result metadata, which will be uploaded to firebase upon successful packing. If the packing is local, the value will be null

with @ascibisz

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

Steps to Verify:

  1. use API gateway to trigger a packing batch job (currently, only Alli and Ruge have access, but I'll attach screenshots to demonstrate the steps )
  2. wait 30-60sec for the job to run
  3. check the results collection in staging firebase database - recipe metadata should display the correct batch job id
  4. optional: pack a recipe locally - the batch_job_id field should also be present but with a null value

Screenshots (optional):

Show-n-tell images/animations here
Step 1 (API Gateway): use API gateway to trigger a packing batch job
Screenshot 2024-10-28 at 3 27 22 PM

Step 2 (AWS Batch): wait for the batch job to run
Screenshot 2024-10-28 at 3 29 44 PM

Step 3 (Firebase): batch job id will be added to the metadata and uploaded to firebase
Screenshot 2024-10-28 at 3 30 38 PM

Copy link

github-actions bot commented Oct 28, 2024

Packing analysis report

Analysis for packing results located at cellpack/tests/outputs/test_spheres/spheresSST

Ingredient name Encapsulating radius Average number packed
ext_A 25 236.0

Packing image

Packing image

Distance analysis

Expected minimum distance: 50.00
Actual minimum distance: 50.01

Ingredient key Pairwise distance distribution
ext_A Distance distribution ext_A

@rugeli rugeli changed the title Feature/job add batch_job_id to result metadata Oct 28, 2024
@rugeli rugeli requested review from ascibisz and meganrm October 28, 2024 22:35
@@ -1398,23 +1398,35 @@ def raycast_test(self, obj, start, end, length, **kw):
def post_and_open_file(self, file_name, open_results_in_browser=True):
simularium_file = Path(f"{file_name}.simularium")
url = None
job_id = os.environ.get("AWS_BATCH_JOB_ID", None)
if job_id:
print(f"Batch Job ID: {job_id}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: we can probably remove lines 1402 and 1403, this was just for debugging

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good catch! Thanks!

Copy link
Collaborator

@ascibisz ascibisz left a comment

Choose a reason for hiding this comment

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

LGTM!

@rugeli rugeli merged commit cdef941 into feature/pass-recipe-to-docker Nov 12, 2024
2 checks passed
@rugeli rugeli deleted the feature/job-id branch November 12, 2024 22:36
rugeli added a commit that referenced this pull request Nov 15, 2024
* draft aws cli inside of docker container

* access aws s3 with saved cred

* update readme

* update docker commands

* update readme

* update readme

* add parameter for when we're using docker to default to using the staging firebase db

* read recipe and config in from docker run params

* use dockerignore file to prevent bloat in docker image

* add `batch_job_id` to result metadata (#295)

* retrieve job id

* formatting

* add a bucket for batch jobs

* formatting

* remove print statement and add comments

---------

Co-authored-by: Ruge Li <[email protected]>
Co-authored-by: Ruge Li <[email protected]>
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.

3 participants