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

set staging as the default database for S3 metadata storage #234

Merged
merged 4 commits into from
Mar 21, 2024

Conversation

rugeli
Copy link
Collaborator

@rugeli rugeli commented Feb 28, 2024

Problem

Merge #230 before this PR

What is the problem this work solves, including
closes #233 #229

Solution

What I/we did to solve this problem

  • Added logic to initialize firebase handler with the default_db parameter if provided and applicable.
  • Set default_db to staging in store_metadata() within the simularium helper.

Type of change

Please delete options that are not relevant.

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

Change summary:

  • The system will now store S3 metadata in our staging firebase by default for each local packing

Steps to Verify:

  1. run any local recipe e.g pack -r examples/recipes/v2/one_sphere.json -c examples/packing-configs/run.json, it shouldn't prompt you to choose a database
  2. upload a local recipe and run the uploaded recipe to test if this change breaks anything

Screenshots (optional):

Show-n-tell images/animations here
Screenshot 2024-02-27 at 7 11 22 PM

@codecov-commenter
Copy link

codecov-commenter commented Feb 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.75%. Comparing base (c78b89b) to head (6330761).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #234   +/-   ##
=======================================
  Coverage   98.75%   98.75%           
=======================================
  Files          19       19           
  Lines         563      563           
=======================================
  Hits          556      556           
  Misses          7        7           

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

Copy link

github-actions bot commented Feb 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 requested review from mogres and meganrm February 28, 2024 17:42
@rugeli rugeli changed the title set staging as the default database in store_metadata set staging as the default database for S3 metadata storage Feb 28, 2024
Copy link
Collaborator

@mogres mogres left a comment

Choose a reason for hiding this comment

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

Running into an error

An error occurred while storing the file out/one_sphere/jitter/results_one_sphere_default-run_1.0.0_seed_0.simularium to S3: Failed to initialize a certificate credential. Caused by: "None could not be converted to bytes"

while packing the one_sphere recipe in the PR description.

I do have firebase credentials and they are being loaded:

2024-02-28 15:36:40,857 - botocore.credentials - INFO - credentials.py:1255 -                 load(): Found credentials in shared credentials file: ~/.aws/credentials

@rugeli
Copy link
Collaborator Author

rugeli commented Feb 29, 2024

Running into an error

An error occurred while storing the file out/one_sphere/jitter/results_one_sphere_default-run_1.0.0_seed_0.simularium to S3: Failed to initialize a certificate credential. Caused by: "None could not be converted to bytes"

while packing the one_sphere recipe in the PR description.

I do have firebase credentials and they are being loaded:

2024-02-28 15:36:40,857 - botocore.credentials - INFO - credentials.py:1255 -                 load(): Found credentials in shared credentials file: ~/.aws/credentials

The loaded botocore credential is for S3, and the error you ran into is from the Google Auth library for Firebase. My guess is that it has something to do with the .env file and how the token is stored, if you could DM me the full error log, we can start ruling out possibilities from there.

And I definitely see how the error messaging could be clearer and more helpful here. I’ll have another pr to explicitly refine error handling for aws and firebase. Thank you for testing and giving feedback!

@mogres
Copy link
Collaborator

mogres commented Feb 29, 2024

Running into an error

An error occurred while storing the file out/one_sphere/jitter/results_one_sphere_default-run_1.0.0_seed_0.simularium to S3: Failed to initialize a certificate credential. Caused by: "None could not be converted to bytes"

while packing the one_sphere recipe in the PR description.
I do have firebase credentials and they are being loaded:

2024-02-28 15:36:40,857 - botocore.credentials - INFO - credentials.py:1255 -                 load(): Found credentials in shared credentials file: ~/.aws/credentials

The loaded botocore credential is for S3, and the error you ran into is from the Google Auth library for Firebase. My guess is that it has something to do with the .env file and how the token is stored, if you could DM me the full error log, we can start ruling out possibilities from there.

And I definitely see how the error messaging could be clearer and more helpful here. I’ll have another pr to explicitly refine error handling for aws and firebase. Thank you for testing and giving feedback!

Oops, just noticed the loaded credentials were for aws and not Firebase. My apologies! Will follow up in DM

@rugeli rugeli requested a review from mogres February 29, 2024 21:04
Copy link
Collaborator

@mogres mogres left a comment

Choose a reason for hiding this comment

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

I am able to run recipes locally and from firebase, and am also able to upload to the staging database. Thanks for the updates!

@rugeli rugeli merged commit 467d341 into main Mar 21, 2024
7 checks passed
@rugeli rugeli deleted the feature/default-stag-for-S3 branch March 21, 2024 18:18
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.

Proposal: defaulting S3 metadata uploads to staging database
4 participants