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

APPSRE-5046 generated types for service_dependencies #2495

Merged

Conversation

fishi0x01
Copy link
Contributor

@fishi0x01 fishi0x01 commented Jun 24, 2022

We want to introduce saas_file_v2 in service_dependencies checks.

  1. we add our new code generator to reconcile.
  2. we include an introspection.json of our schema. That json is required in order to infer types. The json is generated by the code generator via make gql-introspection. Note, that at a later step we might decide to package the introspection schema in a different place/repo. This is the first iteration.
  3. we place our gql query into a dedicated .gql file.
  4. the code generator generates corresponding data classes via make gql-query-classes.
  5. we migrate existing logic to use the new generated types

From now on, any schema change will also require:

make gql-introsprection
make gql-query-classes

If the schema is not compatible in any way, we will see errors during type checking phase.
A check that the introspection.json and generated classes are up-to-date with the latest schema should be part of CI in later step.

This will be merged after we merged our design doc for the code generator. Design doc is merged.

This PR gives a very practical insight on how type generation will look on an existing integration.

A follow-up PR will then introduce saas_file_v2 to the query.

@fishi0x01 fishi0x01 force-pushed the APPSRE-5046_service-deps-saas-file-v2 branch 3 times, most recently from 3aa806c to a290f1b Compare June 24, 2022 10:51
@fishi0x01 fishi0x01 force-pushed the APPSRE-5046_service-deps-saas-file-v2 branch from 61e9919 to ae67231 Compare July 4, 2022 09:17
pyproject.toml Outdated Show resolved Hide resolved
@fishi0x01
Copy link
Contributor Author

@geoberle brought up an interesting idea on slack: generating a convenience function to load the query string.

def load_query_string() -> str:
  with open('path_to_file.gql', r) as f:
    return f.read()

That way we dont have to mess around with the file paths. I will add this to the generator now.

@fishi0x01
Copy link
Contributor Author

fishi0x01 commented Jul 4, 2022

Convenience function to load query strings added here: app-sre/qenerate#21

I.e., @geoberle 's idea is implemented now

@fishi0x01 fishi0x01 force-pushed the APPSRE-5046_service-deps-saas-file-v2 branch from 7029dc6 to d648f86 Compare July 4, 2022 14:07
Copy link
Member

@chassing chassing left a comment

Choose a reason for hiding this comment

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

LGTM

@fishi0x01 fishi0x01 force-pushed the APPSRE-5046_service-deps-saas-file-v2 branch from d648f86 to 32a16cd Compare July 5, 2022 06:40
@fishi0x01 fishi0x01 merged commit d34cc46 into app-sre:master Jul 5, 2022
@fishi0x01 fishi0x01 deleted the APPSRE-5046_service-deps-saas-file-v2 branch July 5, 2022 06:46

from reconcile.utils import gql
from reconcile import queries
from reconcile.utils.external_resources import (
get_provision_providers,
Copy link
Contributor

Choose a reason for hiding this comment

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

get_provision_providers seems to not be used anywhere anymore (except in a test file), can we remove it and clean the code further ?

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