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

split airbyte protocol generated structs out of base_python #721

Merged
merged 18 commits into from Oct 28, 2020

Conversation

cgardens
Copy link
Contributor

@cgardens cgardens commented Oct 28, 2020

What

  • See issue for more color: Make it possible to depend on python airbyte protocol generated structs without running entrypoint. #718
  • Splits the generated airbyte protocol structs into their own lib so that they can be depended upon without needing to depend on base_python.
  • after this PR:
    • airbyte-protocol - lib with structs
    • base-python - the base python stuff
  • try to get all of the dependsOn right in gradle for the python modules. in some cases we were missing dependsOn statements leading to non-deterministic behavior. honestly not sure that i got them all yet.

@cgardens cgardens changed the base branch from cgardens/python_to_standard_test to master October 28, 2020 03:21
@@ -1 +0,0 @@
../../bases/base-singer/base_singer
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jrhizor i ended up deleting a ton of these symlinks. do you know why these were here? or did they just sneak in?

Copy link
Contributor

Choose a reason for hiding this comment

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

It was on purpose for DX: #716 (comment)

@@ -31,7 +31,7 @@
author_email="contact@airbyte.io",
url="https://github.com/airbytehq/airbyte",
packages=setuptools.find_packages(),
install_requires=["airbyte_protocol"],
install_requires=["airbyte-protocol"],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

apparently python will accept either of these.... _ or -. normalizing to - as i think it should be referring to the package name.

@@ -0,0 +1 @@
airbyte_protocol/models/yaml
Copy link
Contributor

Choose a reason for hiding this comment

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

There isn't anything in this location. This .gitignore file can be deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks.

SOFTWARE.
"""

from airbyte_protocol.entrypoint import main
Copy link
Contributor

Choose a reason for hiding this comment

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

This is in the base, not airbyte-protocol? Can we just delete this file? I don't think we need it for a library class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed.

@cgardens cgardens merged commit 3ef2baa into master Oct 28, 2020
@cgardens cgardens deleted the cgardens/split_out_structs branch October 28, 2020 17:03
Copy link
Contributor

@michel-tricot michel-tricot left a comment

Choose a reason for hiding this comment

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

great!

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.

None yet

4 participants