feat(Airbyte-ci): add command generate-erd-schema #43310

artem1205 · 2024-08-05T20:59:57Z

What

add new command in airbyte-ci connectors generate-erd and addresses https://github.com/airbytehq/airbyte-internal-issues/issues/9172

How

report /steps example

Review guide

airbyte-ci/connectors/erd/relationships.py: how we manage relationships
airbyte-ci/connectors/erd/dbml_assembler.py: how we generate the dbml file
airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd: the command logic
the rest

User Impact

Given a couple of pre-requisite, users will now be able to generate ERDs in dbdocs using airbyte-ci. For more information, read the README

Can this PR be safely reverted and rolled back?

YES 💚
NO ❌

Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

[skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

vercel · 2024-08-05T21:00:00Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	⬜️ Ignored (Inspect)	Visit Preview		Aug 20, 2024 3:06pm

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd_schema/pipeline.py

[skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

add parsers for discover, gemini md output; [skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

[skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

… adding tests

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd_schema/pipeline.py

maxi297 · 2024-08-15T18:37:33Z

airbyte-ci/connectors/pipelines/pyproject.toml

@@ -18,6 +18,9 @@ more-itertools = "^8.11.0"
 docker = "^7"
 semver = "^3"
 airbyte-protocol-models = "*"
+# In an ideal world, we would import the version of airbyte-ci for the connector airbyte-ci will interact with
+# Also note that the version that this will currently import is <2 because of the version clash with pydantic
+airbyte-cdk = "*"


Just a comment to highlight this comment

This has been moved outside of the pipelines and therefore is not relevant anymore. See #43310 (review) for more details

maxi297

Recreating comments as GitHub has flagged them as outdated

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/pipeline.py

maxi297 · 2024-08-15T18:42:03Z

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/pipeline.py

+            .with_exec(["npm", "install", "-g", "dbdocs"])
+            .with_env_variable("DBDOCS_TOKEN", DBDOCS_TOKEN)
+            .with_workdir("/airbyte_dbdocs")
+            .with_new_file("/airbyte_dbdocs/dbdocs.dbml", contents=source_dbml_content)


I've seen some weird cases where the dbdocs diagram wasn't updated as part of the airbyte-ci command but running the same command locally would update the dbdocs project. Can we trust that if source_dbml_content is different, the cache won't be triggered or is it safer to add .with_env_variable("CACHEBUSTER", str(uuid.uuid4()))?

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/pipeline.py

artem1205

LGTM

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/relationships.py

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/commands.py

alafanechere

I'm requesting change to ask you to follow the airbyte-ci is just an orchestrator principle

I suggest declaring ERD generation logic in an independent python package which would take inputs passed from airbyte-ci.
airbyte-ci would consume your package output and export back the ERDs to the connector container:

Your airbyte-ci step would:

Run discover on the connector and capture the catalog
Mount the catalog to the container in which your package will be installed
Run the erd generation command in your package
Export the generated ERD files back to the host in the connector folder

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/commands.py

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/dbml_assembler.py

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/pipeline.py

…ariables

alafanechere

Disclaimer: I did not review the internal logic of your ERD packages, I focused on airbyte-ci.

I've a lot of minor suggestion but nothing blocking! Thanks for following my refactoring original suggest, you got this!

I'm not sure if you want to version control the ERD.
If you want: the current logic is 🆗 .

If you don't: I think it's better to pass the generated file from the erd container to your UploadToDbDocs step. It will avoid an intermediate "write to host FS / repo" operation.

If you still want to keep the same flow but don't plan on committing generated files (as it could be helpful for debugging to have the generated file on your FS), please add a .gitignore rule.

airbyte-ci/connectors/erd/src/erd/erd_service.py

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/context.py

airbyte-ci/connectors/erd/pyproject.toml

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/pipeline.py

maxi297 · 2024-08-19T15:40:10Z

If you still want to keep the same flow but don't plan on committing generated files (as it could be helpful for debugging to have the generated file on your FS), please add a .gitignore rule.

It has been discussed that we should have to ability to commit it so that we can see the different versions and debug. I'm not sure if we will do this at first but I want us to have the ability to do so.

artem1205 added 2 commits August 5, 2024 22:56

Airbyte CI: add erg generator

1a8c1d9

Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

Airbyte CI: add note

bd7c85b

[skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

artem1205 assigned maxi297 and artem1205 Aug 5, 2024

natikgadzhi reviewed Aug 5, 2024

View reviewed changes

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd_schema/pipeline.py Outdated Show resolved Hide resolved

artem1205 and others added 9 commits August 6, 2024 12:22

Airbyte CI: add google gen ai deps

9f75e91

[skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

Airbyte CI: ref;

a43b73c

add parsers for discover, gemini md output; [skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

Airbyte CI: add UploadDbmlSchema step

cfa2f30

[skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

Airbyte CI: add TODO

fec71f2

[skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

Airbyte CI: add pydbml

549cc40

[skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

Airbyte CI: add generate_dbml step

94477ae

[skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

Airbyte CI: ref

a2df90b

[skip ci] Signed-off-by: Artem Inzhyyants <artem.inzhyyants@gmail.com>

Supporting manual override, creating files in erd folder, cleanup and…

4b82618

… adding tests

Adding documentation and improving naming

2e2e7f6

maxi297 requested a review from alafanechere August 15, 2024 18:36

maxi297 reviewed Aug 15, 2024

View reviewed changes

maxi297 marked this pull request as ready for review August 15, 2024 18:36

maxi297 requested a review from a team as a code owner August 15, 2024 18:36

maxi297 reviewed Aug 15, 2024

View reviewed changes

artem1205 commented Aug 16, 2024

View reviewed changes

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/relationships.py Outdated Show resolved Hide resolved

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/generate_erd/commands.py Outdated Show resolved Hide resolved

alafanechere requested changes Aug 16, 2024

View reviewed changes

maxi297 and others added 5 commits August 16, 2024 14:40

Extract erd logic into its own module

a7f625d

Get secrets through local secret store and pass secrets through env v…

48fb845

…ariables

Revert dependency changes

ece0f9a

Merge branch 'master' into artem1205/airbyte_ci_erd_gen

a84b672

code review artem

5990e67

maxi297 requested a review from alafanechere August 16, 2024 19:49

format/lint/typecheck

81b095a

alafanechere approved these changes Aug 19, 2024

View reviewed changes

maxi297 and others added 4 commits August 19, 2024 13:38

Code review part 2

0a4ce68

format/lint

288da20

type checking

cd73870

Merge branch 'master' into artem1205/airbyte_ci_erd_gen

f9f3553

vercel bot deployed to Preview August 19, 2024 18:44 View deployment

maxi297 added 3 commits August 20, 2024 09:02

Fix folder issues

417ba17

Fix low-code without schema_loader

734d044

format

eea144f

maxi297 merged commit 9603c3c into master Aug 20, 2024
33 checks passed

maxi297 deleted the artem1205/airbyte_ci_erd_gen branch August 20, 2024 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Airbyte-ci): add command generate-erd-schema #43310

feat(Airbyte-ci): add command generate-erd-schema #43310

feat(Airbyte-ci): add command generate-erd-schema #43310

feat(Airbyte-ci): add command generate-erd-schema #43310

Conversation

What

How

Review guide

User Impact

Can this PR be safely reverted and rolled back?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment