🐙 octavia-cli: add generate for connections #10809

marcosmarxm · 2022-03-02T21:06:17Z

What

Describe what the change is solving
It helps to add screenshots if it affects the frontend.

How

Describe the solution

🚨 User Impact 🚨

Are there any breaking changes? What is the end result perceived by the user? If yes, please merge this PR with the 🚨🚨 emoji so changelog authors can further highlight this if needed.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

New Connector

Community member or Airbyter

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the connector is published, connector added to connector index as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Updating a connector

Community member or Airbyter

Grant edit access to maintainers (instructions)
Secrets in the connector's spec are annotated with airbyte_secret
Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
Code reviews completed
Documentation updated
- Connector's README.md
- Connector's bootstrap.md. See description and examples
- Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

Create a non-forked branch based on this PR and test the below items on it
Build is successful
Credentials added to Github CI. Instructions.
/test connector=connectors/<name> command is passing
New Connector version released on Dockerhub by running the /publish command described here
After the new connector version is published, connector version bumped in the seed directory as described here
Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here

Connector Generator

Issue acceptance criteria met
PR name follows PR naming conventions
If adding a new generator, add it to the list of scaffold modules being tested
The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

octavia-cli/Dockerfile

octavia-cli/README.md

alafanechere · 2022-03-04T09:45:22Z

octavia-cli/octavia_cli/generate/connection.py

+from airbyte_api_client.model.source_id_request_body import SourceIdRequestBody
+
+
+class Connection:


Sorry but I've a strong refacto suggestion 😄 :

I think that the class you describe here is not a Connection but a Catalog

I think that catalog should be attributes of sources (in the API discover_schema is called within the source endpoint: /v1/sources/discover_schema)

In the octavia apply PR I created Source objects (here)

My suggestion is to add a catalog attribute to the Source and publically expose this attribute, enabling this kind of interactions:

my_source = Source(api_client, ...) my_source.catalog.streams

This will be a way to explicitly declare the dependency between a source (or destination) existence before generating the connection YAML file.
I think this also can lead to the following change of your command function (octavia generate connection):

octavia generate connection --source ./path/to/my/source.yaml --destination ./path/to/my/destination.yaml my_connection_name

The command will do something like:

import octavia_cli.apply.resources source = resources.factory(api_client, workspace_id, source_file_path) destination = resources.factory(api_client, workspace_id, destination_file_path) if not (source.was_created and destination.was_created): raise resources.NonExistingRessourceError("...") connection_renderer = ConnectionRenderer(connection_name, source.resource_id, destination.resource_id, source.catalog.streams) connection_renderer.write_yaml(".")

Gotcha! Makes sense

octavia-cli/octavia_cli/templates/connection.yaml.j2

octavia-cli/octavia_cli/generate/commands.py

alafanechere · 2022-03-04T10:07:58Z

octavia-cli/octavia_cli/generate/commands.py

+@click.argument("source_id", type=click.STRING)
+@click.argument("destination_id", type=click.STRING)


I think that this command should rather take the source and destination YAML file path. I explain why in my next comment :)

alafanechere · 2022-03-04T10:12:26Z

octavia-cli/octavia_cli/templates/connection.yaml.j2

+name: {{ resource_name }}
+source_id: {{ source_id }}
+destination_id: {{ destination_id }}


Suggested change

name: {{ resource_name }}

source_id: {{ source_id }}

destination_id: {{ destination_id }}

resource_name: mock_connection

definition_type: connection

source_id: mock_source

destination_id: mock_destination

Could you please try to mimic the top of the other template?

Maybe we can create template composition with Jinja to make source_or_destination.yaml.j2 and connection.yaml.j2 share the same headers.

octavia-cli/octavia_cli/generate/renderers.py

marcosmarxm added 3 commits February 25, 2022 19:47

create connection cli command

623d6f8

add connection.yaml template

dab0307

add template file

a85dd65

marcosmarxm requested a review from alafanechere March 2, 2022 21:06

marcosmarxm temporarily deployed to more-secrets March 2, 2022 21:07 Inactive

marcosmarxm added 2 commits March 3, 2022 11:07

remove unused vars and imports

eb800bf

create group for generate command

fae98ea

marcosmarxm temporarily deployed to more-secrets March 3, 2022 14:32 Inactive

alafanechere suggested changes Mar 4, 2022

View reviewed changes

alafanechere reviewed Mar 4, 2022

View reviewed changes

alafanechere mentioned this pull request Mar 6, 2022

[EPIC]: Octavia CLI alpha #10704

Closed

merge master

095984a

marcosmarxm temporarily deployed to more-secrets March 7, 2022 11:39 Inactive

correct connection template

d0a547c

marcosmarxm marked this pull request as ready for review March 7, 2022 12:51

marcosmarxm requested a review from alafanechere March 7, 2022 12:51

marcosmarxm temporarily deployed to more-secrets March 7, 2022 12:52 Inactive

implement suggestions

da88602

alafanechere temporarily deployed to more-secrets March 7, 2022 15:06 Inactive

implement suggestions

42ba11f

alafanechere temporarily deployed to more-secrets March 7, 2022 16:41 Inactive

fix existing tests

f05d24d

marcosmarxm changed the title ~~octavia-cli: add generate for connections~~ 🐙 octavia-cli: add generate for connections Mar 7, 2022

alafanechere added 2 commits March 8, 2022 09:15

test resources

b2981d1

test renderer

37b28f9

alafanechere added 2 commits March 8, 2022 11:45

revert unrelated changes

910e052

clean

15ea2be

alafanechere temporarily deployed to more-secrets March 8, 2022 10:48 Inactive

clean

0a2d90d

alafanechere temporarily deployed to more-secrets March 8, 2022 10:52 Inactive

add required resource_name field to template

31e515a

alafanechere temporarily deployed to more-secrets March 8, 2022 11:15 Inactive

alafanechere added 3 commits March 8, 2022 14:49

undo snakecasing + delete jsonschema

8c4ceee

add source_id destination_id to yaml top

81e04b7

fix

7462827

alafanechere temporarily deployed to more-secrets March 8, 2022 19:12 Inactive

alafanechere temporarily deployed to more-secrets March 8, 2022 19:13 Inactive

add json schema in connection integration test

fdd7909

alafanechere temporarily deployed to more-secrets March 9, 2022 12:41 Inactive

marcosmarxm commented Mar 9, 2022

View reviewed changes

octavia-cli/octavia_cli/generate/renderers.py Outdated Show resolved Hide resolved

Merge branch 'master' into marcos/octavia-generate-connection

be2a910

alafanechere temporarily deployed to more-secrets March 9, 2022 12:44 Inactive

make write_yaml a base method + implement _render

d184a88

alafanechere approved these changes Mar 9, 2022

View reviewed changes

alafanechere temporarily deployed to more-secrets March 9, 2022 13:05 Inactive

marcosmarxm merged commit 8f49e19 into master Mar 9, 2022

marcosmarxm deleted the marcos/octavia-generate-connection branch March 9, 2022 13:06

alafanechere linked an issue Mar 9, 2022 that may be closed by this pull request

🐙 octavia-cli: Generate YAML template file for connections #10706

Closed

octavia-squidington-iii mentioned this pull request Mar 9, 2022

Bump Airbyte version from 0.35.47-alpha to 0.35.48-alpha #11007

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

🐙 octavia-cli: add generate for connections #10809

🐙 octavia-cli: add generate for connections #10809

Uh oh!

marcosmarxm commented Mar 2, 2022

Uh oh!

Uh oh!

Uh oh!

alafanechere Mar 4, 2022

Uh oh!

marcosmarxm Mar 4, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alafanechere Mar 4, 2022

Uh oh!

alafanechere Mar 4, 2022

Uh oh!

alafanechere Mar 4, 2022

Uh oh!

Uh oh!

Uh oh!

		from airbyte_api_client.model.source_id_request_body import SourceIdRequestBody


		class Connection:

		@click.argument("source_id", type=click.STRING)
		@click.argument("destination_id", type=click.STRING)

-name: {{ resource_name }}
-source_id: {{ source_id }}
-destination_id: {{ destination_id }}
+resource_name: mock_connection
+definition_type: connection
+source_id: mock_source
+destination_id: mock_destination

🐙 octavia-cli: add generate for connections #10809

🐙 octavia-cli: add generate for connections #10809

Uh oh!

Conversation

marcosmarxm commented Mar 2, 2022

What

How

Recommended reading order

🚨 User Impact 🚨

Pre-merge Checklist

Community member or Airbyter

Airbyter

Community member or Airbyter

Airbyter

Tests

Uh oh!

Uh oh!

Uh oh!

alafanechere Mar 4, 2022

Choose a reason for hiding this comment

Uh oh!

marcosmarxm Mar 4, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alafanechere Mar 4, 2022

Choose a reason for hiding this comment

Uh oh!

alafanechere Mar 4, 2022

Choose a reason for hiding this comment

Uh oh!

alafanechere Mar 4, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!