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

connector-templates: move generated code from src so airbyte-ci builds it #36428

Conversation

natikgadzhi
Copy link
Contributor

@natikgadzhi natikgadzhi commented Mar 24, 2024

What

For newly generated python and low-code connectors, move sources from src to source_%CONN_NAME% so airbyte-ci build can pick them up correctly. Closes #35893.

What changed?

  • Moved the sources from src
  • Updated package paths in pyproject.toml templates

How to test?

You can make a new connector with ./generate.sh and verify that it works with Poetry and that it builds with airbyte-ci build --name source-test


This commit cleans up connector generator, specifically:
- Removes java sources and destinations from plopfile
- Removes singer source from plopfile
- Removes generic and python (non-http) source from plopfile
- Cleans up documentation
Copy link

vercel bot commented Mar 24, 2024

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Mar 24, 2024 3:09pm

Copy link
Contributor Author

natikgadzhi commented Mar 24, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

Join @natikgadzhi and the rest of your teammates on Graphite Graphite

@natikgadzhi natikgadzhi marked this pull request as ready for review March 24, 2024 15:12
@natikgadzhi natikgadzhi requested a review from a team as a code owner March 24, 2024 15:12
@natikgadzhi natikgadzhi changed the title connector-generator: move generated code from src so airbyte-ci builds it connector-templates: move generated code from src so airbyte-ci builds it Mar 24, 2024
@stephane-airbyte stephane-airbyte changed the base branch from ng/connector-generator-cleanup to master March 24, 2024 16:58
@natikgadzhi natikgadzhi changed the base branch from master to ng/connector-generator-cleanup March 24, 2024 17:54
@natikgadzhi natikgadzhi changed the base branch from ng/connector-generator-cleanup to master March 24, 2024 17:54
@natikgadzhi natikgadzhi changed the base branch from master to ng/connector-generator-cleanup March 24, 2024 18:10
@natikgadzhi natikgadzhi changed the base branch from ng/connector-generator-cleanup to master March 24, 2024 18:10
Copy link
Contributor

@alafanechere alafanechere left a comment

Choose a reason for hiding this comment

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

@natikgadzhi I originally wanted to make connectors follow the src layout as its a good python practice. I might have done it too agressively and the build logic in airbyte-ci might not support it correctly indeed.
I think the bug can be solved with a one line change here: including src instead of connector_snake_case_name. But if we want to support flat and srclayout the logic has to change a bit more. So I'll 👍 this one 😄

Can you please re-generate the scaffolds?
(or delete them? - I think no test are actually checking scaffolds are consistent with the generated code anymore).

 ./gradlew --no-daemon :airbyte-integrations:connector-templates:generator:generateScaffolds

I noticed you also deleted the java + configuration-based generator.
I'd suggest doing it in a separate PR in case we'd want to revert it.
And you should also remove source-singer as it's legacy too. I'd also challenge the usefulness source-python generator. If anyone wants to build a non-http python connector they can start off from the http one and remove the http related code.

@@ -12,7 +12,7 @@ readme = "README.md"
documentation = "https://docs.airbyte.com/integrations/sources/{{dashCase name}}"
homepage = "https://airbyte.com"
repository = "https://github.com/airbytehq/airbyte"
packages = [ { include = "source_{{snakeCase name}}", from="src"}, {include = "main.py", from = "src"} ]
packages = [ { include = "source_{{snakeCase name}}" }, {include = "main.py" } ]
Copy link
Contributor

Choose a reason for hiding this comment

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

This line can be completly remove from all pyproject.toml if we're using the flat layout instead of the src layout

@natikgadzhi natikgadzhi merged commit 0e9bdf3 into master Mar 25, 2024
31 checks passed
@natikgadzhi natikgadzhi deleted the 03-24-connector-generator_move_generated_code_from_src_so_airbyte-ci_builds_it branch March 25, 2024 17:01
Copy link
Contributor Author

Merge activity

@natikgadzhi
Copy link
Contributor Author

Whooops, this got merged with Graphite automatically on approval, but I will follow-up on @alafanechere's asks:

  • Removing source-singer is here: connector-templates: Cleanup connector generator #36427 — can you please give this one thumbs up so it also gets merged? They were stacked together, so I think it will need to be rebased.
  • Can you please re-generate the scaffolds? — will do in a separate PR then!

@sukolenvo
Copy link

Seems like java connector has been removed in this PR. Tried to follow steps and it didn't work: https://docs.airbyte.com/connector-development/tutorials/building-a-java-destination

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.

Connector generator: Investigate build issues
3 participants