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

feat: API V2 - Local dev only for now #203

Merged
merged 105 commits into from
Aug 29, 2024
Merged

feat: API V2 - Local dev only for now #203

merged 105 commits into from
Aug 29, 2024

Conversation

jgadling
Copy link
Contributor

@jgadling jgadling commented Aug 13, 2024

This PR adds a v2 GraphQL API based on the work in the platformics repo. For now it includes its own (slightly modified) version of the base platformics codebase, while we wait for several changes to the base library to be merged upstream. We'll eventually pull in the platformics library from pypi to avoid maintaining multiple copies.

The schema is in apiv2/schema/schema.yaml and the GraphQL API that gets generated is relatively similar to the interface presented by Hasura, except that all 1:many relationships implement the Relay connection spec. That means if we have an API V1 query that looks like this:

query MyQuery {
  datasets {
    id
    runs {
      id
    }
  }
}

It will look like this in API V2:

query MyQuery {
  datasets {
    id
    runs {
      edges {
        node {
          id
        }
      }
    }
  }
}

Currently this new API isn't integrated into the same local dev environment as the rest of the containers when running make init from the root of the repo. I'm hoping to address that in a later PR.


...
else:
File = "File"
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: is File being used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. I'm not sure why ruff isn't removing these unused imports but they're harmless.

.github/workflows/staging-deploy.yaml Outdated Show resolved Hide resolved
services:
apiv2:
image:
repository: 533267185808.dkr.ecr.us-west-2.amazonaws.com/core-platform/cryoet-data-portal-backend/apiv2
Copy link
Contributor

Choose a reason for hiding this comment

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

It is interesting that we are using a different account for storing the images. Is there any specific reason on why we aren't storing it in the relevant account?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just the way the argus team has things set up by default :'( eventually I'd like it to pull from github's image repo, but it doesn't seem worth fighting with right now.

.pre-commit-config.yaml Show resolved Hide resolved
Comment on lines +2 to +11
name = "platformics"
version = "0.1.0"
description = "Platformics bio entities framework"
authors = ["CZI Infectious Disease Team <[email protected]>"]
license = "MIT License"
readme = "README.md"
packages = [{include = "platformics"}]

[tool.poetry.scripts]
platformics = 'platformics.cli.main:cli'
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we rename this? 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ehhh, we're eventually going to be using the upstream platformics package and this stuff can go away, but in the meantime it's 🤷‍♀️ ... we can clean it up when we delete the platformics directory here?

apiv2/graphql_api/schema.graphql Outdated Show resolved Hide resolved
Comment on lines +295 to +308
deposition_types_enum:
name: deposition_types_enum
description: Types of data a deposition has
from_schema: cdp-dataset-config
permissible_values:
annotation:
text: annotation
description: The deposition comprises of new annotations for existing datasets
dataset:
text: dataset
description: The deposition comprises of new dataset(s).
tomogram:
text: tomogram
description: The deposition comprises of new tomograms for existing datasets
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel this shouldn't be an enum at the db level, because in the case we want to have a combination of tomogram and annotation, we would have to create a new type.

Copy link
Contributor Author

@jgadling jgadling Aug 29, 2024

Choose a reason for hiding this comment

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

We have a 1:many relationship between depositions and this deposition_types_enum via the DepositionTypes type!

apiv2/schema/schema.yaml Outdated Show resolved Hide resolved
apiv2/schema/schema.yaml Outdated Show resolved Hide resolved
@jgadling jgadling merged commit b8330bc into main Aug 29, 2024
10 checks passed
@jgadling jgadling deleted the jgadling/apiv2 branch August 29, 2024 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants