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

refactor(breadbox): Service layer proposal #93

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

snwessel
Copy link
Member

Following up from discussion here and in person:

@pgm and @jessica-cheng it seemed like you were both enthusiastic about creating a "service layer" in breadbox so that we have a more typical 3-tiered setup. I personally am very excited about this because I'm finding the existing setup increasingly confusing and I think a service layer would be helpful for the DE2 support I am currently working on. I wanted to ask what that would look like in your minds, but I thought it might be easier to discuss a tangible example.

I am not attached to anything here, so feel free to disagree if you see something you don't like (for example, I'm not sure my naming is the best), and we can talk more at the breadbox meeting tomorrow if that's easier than giving feedback here.

Currently, breadbox has two "tiers":

  • api (endpoint definitions)
  • crud (contains almost all of our functions that do any data reading/writing, not just low-level crud operations)

I would propose our 3 tiers look something like:

  • api - only contains endpoint definitions (this is already the case)
  • service - has higher-level logic about how the app should function. Nothing here should include SQLAlchemy queries.
  • crud - reusable functions which are directly querying the database (all SQLAlchemy queries would still go here)

We could slowly move service-type-logic out of the crud layer to get things into a more organized state. I did some low-hanging fruit here and set up a service module with 3 files, each containing a few functions:

  • labels.py:
    • get_dataset_feature_labels_by_id
    • get_dataset_sample_labels_by_id
    • get_dataset_feature_by_label
    • get_dataset_sample_by_label
  • slice.py:
    • get_slice_data
    • get_labels_for_slice_type
  • data_loading.py:
    • get_subsetted_matrix_dataset_df

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.

1 participant