Move source-declarative-manifest to a standard source, published in step with python cdk #36501

erohmensing · 2024-03-26T19:26:32Z

What

Improve the publishing flow

Remove manage.sh

out of all of its logic that still existed, all it did was build and push the source-declarative-manifest image.
related - remove legacy-publish from the slash commands list - the workflow it would call does not exist anymore (the one which called manage.sh).

Stop spinning up AWS runners for the publish workflow - we previously couldn't move off because some manage.sh stuff would fail.
During the publishing process, just bump the version of source-declarative-manifest via airbyte-ci. The source will be published via the "publish on merge (i.e. publish on push to master)" flow when the version bump is committed. This is because we've moved source-declarative-manifest to be built and published as any other source.

source-declarative-manifest as an airbyte-ci source

Pull the declarative manifest code and its docker image out of the cdk and into the connectors folder
Migrate the image off of the dockerfile and onto our base image (is this necessary? I think it's helpful if we want to stop building connectors separately, but it makes the image bigger)
Migrate it to poetry
Give it a metadata file:

Gets a new definition ID
Is not in any registry
Has a docs file, for changelog entry updating (this helps us keep track of whether we are still updating in step), but this docs page is hidden from the documentation.

Give it a spec - we use the spec operation to check the health of the image. Currently in the platform we hackily bypass spec a bit. It makes sense for this image to have a spec - the spec declares that the user needs to pass an injected manifest via the config.

This source can be built and published via airbyte CI. As of this PR, it will not pass tests in Airbyte CI, because it's not quite the same as an actual source yet. This is okay for now as we will publish it directly (since we are committing the changes instead of merging them via a PR.

vercel · 2024-03-26T19:26:42Z

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

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 5, 2024 4:38pm

erohmensing · 2024-03-26T19:26:47Z

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @erohmensing and the rest of your teammates on Graphite

…blished via airbyte ci. bump version with airbyte-ci command

erohmensing · 2024-04-04T15:15:59Z

...ations/connectors/source-declarative-manifest/unit_tests/test_source_declarative_manifest.py

We don't currently run these anywhere which feels odd (since we don't run the tests). Should we manually do a poetry run pytest unit_tests before we bump the version of declarative manifest?

I doubt we'll think about manually running tests before a version bump. We can either

Find a way to run these tests as part of the version bump flow

Add the tests to the CDK tests suite
Either way, this isn't testing too much complicated logic. It's fine to leave as a follow up item, but let's make sure to create an issue and document the limitation in the readme

I think I'll leave it and document it as part of the "figure out the release strategy" part. Main reasoning:

Adding the tests to the test suite (2) is strange, because the source has its own cdk dependency which won't account for the new changes we're making, which makes testing the old code there more confusing.

Running them as part of the version flow (1) makes more sense for catching errors, because we can test on the new version published to pypi before releasing the new version of source-declarative manifest. It leaves us in a weird state where if the cdk breaks the source somehow, the cdk version gets released and the source version doesn't. Seems like valuable info, but a good way to get versions out of sync and/or have mysterious missing versions of the source when we revert or patch a CDK version to fix it.

I think what this means is that running the tests (ideally better tests, even CAT tests) on source-declarative manifest could be seen as an integration test for the python CDK. Similar to how we in our PR description for CDK changes have "manually test this change on a CDK source" - we should build a process for doing that automatically for the declarative manifest source.

erohmensing · 2024-04-04T15:17:26Z

.github/workflows/publish-cdk-command-manually.yml

-        if: ${{ failure() }}
-        uses: slackapi/slack-github-action@v1.23.0
-        continue-on-error: true
+          context: "master" # TODO change to ref?


I'm not sure if this even matters here because I don't think bump_version cares about context - checking into this. Not sure if we want to pass everything in to this like we do most places I think, or remove the unneeded inputs to avoid confusion (e.g. python registry token is irrelevant as we're not pushing to pypi). I'll probably test out which ones are unnecessary and remove them.

I think it might be useful to use ref for troubleshooting purposes, but I also think we should just remove the parameter if we're unsure it'll be safe / respected

airbyte-integrations/connectors/source-declarative-manifest/README.md

erohmensing · 2024-04-04T15:19:44Z

airbyte-integrations/connectors/source-declarative-manifest/metadata.yaml

@@ -0,0 +1,27 @@
+data:


Metadata in here could be sanity checked. it probably doesn't matter (release stage, ab_internal stuff for example). I generated a new definitionId.

airbyte-integrations/connectors/source-declarative-manifest/pyproject.toml

girarda

looks great! just a few requests for additional docs + some of the unit tests are failing. We should figure out why and fix them

airbyte-integrations/connectors/source-declarative-manifest/metadata.yaml

girarda · 2024-04-04T18:13:36Z

airbyte-integrations/connectors/source-declarative-manifest/metadata.yaml

+      enabled: false
+  releaseDate: 2023-03-01
+  releaseStage: alpha
+  supportLevel: community


not a real problem, but this feels weird. The connector is definitely not maintained by the community

Yeah, I think that's an issue with the "supportLevel"'s meaning in any case - "we support it" does not mean it's certified by our own internal standards. I agree it doesn't matter at the moment since we don't actually show this source in the docs/UI, but we should probably revisit this.

airbyte-integrations/connectors/source-declarative-manifest/metadata.yaml

girarda · 2024-04-04T18:16:23Z

.github/workflows/publish-cdk-command-manually.yml

-        if: ${{ failure() }}
-        uses: slackapi/slack-github-action@v1.23.0
-        continue-on-error: true
+          context: "master" # TODO change to ref?


I think it might be useful to use ref for troubleshooting purposes, but I also think we should just remove the parameter if we're unsure it'll be safe / respected

airbyte-integrations/connectors/source-declarative-manifest/README.md

airbyte-integrations/connectors/source-declarative-manifest/source_declarative_manifest/run.py

...ations/connectors/source-declarative-manifest/unit_tests/test_source_declarative_manifest.py

docs/integrations/sources/low-code.md

aaronsteers · 2024-04-04T22:20:27Z

@erohmensing - I'd be excited to use this with PyAirbyte. Re:

Feature Request: Ability to load custom connectors built using no code builder PyAirbyte#107

Can we add pypi: true in metadata.yml to enable PyPi publishing?

erohmensing · 2024-04-05T15:05:24Z

@aaronsteers I did a pre-release publish and the publish itself seemed to work ok. Don't recommend testing out on that one because it has a pre-poetry CDK version so it might be a bit different, but we can see what happens. Mainly wanted to check that trying to publish it to pypi wouldn't make the whole publish flow start failing.

aaronsteers · 2024-04-05T16:23:55Z

Thanks, @erohmensing! I will wait for this to merge/release before testing, but I'm excited to give it a spin and see if we can leverage it for airbytehq/PyAirbyte#107 in the long run.

cc @bindipankhudi , @natikgadzhi

erohmensing · 2024-04-05T16:41:31Z

/approve-and-merge reason=source-declarative manifest is not expected to pass CATs at the moment. All other CI passing

octavia-approvington · 2024-04-05T16:42:08Z

Our work here is done

octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation CDK Connector Development Kit labels Mar 26, 2024

octavia-squidington-iii added the connectors/source/declarative-manifest label Mar 26, 2024

vercel bot had a problem deploying to Preview March 26, 2024 19:27 Failure

erohmensing force-pushed the ella/manage-sh branch from 7a50195 to d700e7c Compare March 27, 2024 19:34

erohmensing mentioned this pull request Mar 27, 2024

[don't merge] testing publish ci branch #36575

Closed

erohmensing force-pushed the ella/manage-sh branch 2 times, most recently from 9285554 to d10452a Compare April 1, 2024 15:39

erohmensing added 2 commits April 1, 2024 10:41

move source-declarative-manifest out of cdk so it can be built and pu…

8675311

…blished via airbyte ci. bump version with airbyte-ci command

squash changes

7e1ff5d

vercel bot had a problem deploying to Preview April 1, 2024 15:42 Failure

metadata changes

e0fe88d

erohmensing changed the base branch from master to ella/pipelines April 1, 2024 15:47

erohmensing force-pushed the ella/manage-sh branch from d10452a to ff61a65 Compare April 1, 2024 15:47

erohmensing mentioned this pull request Apr 1, 2024

Airbyte CI: rename incorrectly named pipelines #36722

Merged

bump version

039e700

erohmensing force-pushed the ella/pipelines branch from f56c472 to ec250e8 Compare April 1, 2024 15:51

erohmensing force-pushed the ella/manage-sh branch from ff61a65 to 29fbbb7 Compare April 1, 2024 15:51

erohmensing added 6 commits April 1, 2024 12:14

move some stuff to run

f82eae4

move more to run

32bae97

make tests pass

b648f17

fix testS

2ac1a2e

update acceptance test config

ff9358b

remove destination specific stuff

928d5e9

erohmensing force-pushed the ella/pipelines branch from ec250e8 to b1c1b8d Compare April 3, 2024 15:20

erohmensing force-pushed the ella/manage-sh branch 2 times, most recently from a82555c to 8d78e46 Compare April 3, 2024 15:22

vercel bot deployed to Preview April 4, 2024 15:14 View deployment

erohmensing commented Apr 4, 2024

View reviewed changes

erohmensing marked this pull request as ready for review April 4, 2024 15:21

erohmensing requested a review from a team as a code owner April 4, 2024 15:21

erohmensing requested a review from girarda April 4, 2024 15:21

erohmensing added 2 commits April 4, 2024 10:24

format

cba3b74

fix unit tests

3a320b4

girarda approved these changes Apr 4, 2024

View reviewed changes

aaronsteers mentioned this pull request Apr 4, 2024

Feature Request: Ability to load custom connectors built using no code builder airbytehq/PyAirbyte#107

Open

erohmensing force-pushed the ella/manage-sh branch from 5c1a7c2 to 3a320b4 Compare April 4, 2024 22:25

erohmensing added 3 commits April 4, 2024 17:36

hack, i guess

6e1e3bc

smh appease the linter

dca6b41

enable pypi publishing

4e5e41c

erohmensing added 2 commits April 5, 2024 10:20

docstrings, metadata comments, license

9c26936

update readme significantly, docs changelog

5f9f9d9

vercel bot deployed to Preview April 5, 2024 15:40 View deployment

erohmensing force-pushed the ella/manage-sh branch from b671540 to e37b190 Compare April 5, 2024 15:58

try manual

e9b7344

erohmensing force-pushed the ella/manage-sh branch from e37b190 to add01a0 Compare April 5, 2024 16:13

erohmensing force-pushed the ella/manage-sh branch from add01a0 to e9b7344 Compare April 5, 2024 16:23

update all dependencies to latest version

c96ef45

vercel bot deployed to Preview April 5, 2024 16:38 View deployment

octavia-approvington approved these changes Apr 5, 2024

View reviewed changes

octavia-approvington merged commit 51d1353 into master Apr 5, 2024
28 of 30 checks passed

octavia-approvington deleted the ella/manage-sh branch April 5, 2024 16:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move source-declarative-manifest to a standard source, published in step with python cdk #36501

Move source-declarative-manifest to a standard source, published in step with python cdk #36501

erohmensing commented Mar 26, 2024 •

edited

Loading

vercel bot commented Mar 26, 2024 •

edited

Loading

erohmensing commented Mar 26, 2024 •

edited

Loading

erohmensing Apr 4, 2024

girarda Apr 5, 2024

erohmensing Apr 5, 2024

erohmensing Apr 4, 2024

girarda Apr 4, 2024

erohmensing Apr 4, 2024

girarda left a comment

girarda Apr 4, 2024

erohmensing Apr 5, 2024

girarda Apr 4, 2024

aaronsteers commented Apr 4, 2024 •

edited

Loading

erohmensing commented Apr 5, 2024

aaronsteers commented Apr 5, 2024 •

edited

Loading

erohmensing commented Apr 5, 2024

octavia-approvington commented Apr 5, 2024

Move source-declarative-manifest to a standard source, published in step with python cdk #36501

Move source-declarative-manifest to a standard source, published in step with python cdk #36501

Conversation

erohmensing commented Mar 26, 2024 • edited Loading

What

Improve the publishing flow

source-declarative-manifest as an airbyte-ci source

vercel bot commented Mar 26, 2024 • edited Loading

erohmensing commented Mar 26, 2024 • edited Loading

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

girarda left a 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

aaronsteers commented Apr 4, 2024 • edited Loading

erohmensing commented Apr 5, 2024

aaronsteers commented Apr 5, 2024 • edited Loading

erohmensing commented Apr 5, 2024

octavia-approvington commented Apr 5, 2024

erohmensing commented Mar 26, 2024 •

edited

Loading

vercel bot commented Mar 26, 2024 •

edited

Loading

erohmensing commented Mar 26, 2024 •

edited

Loading

aaronsteers commented Apr 4, 2024 •

edited

Loading

aaronsteers commented Apr 5, 2024 •

edited

Loading