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

[FLINK-33557] Externalize Cassandra Python connector code #26

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ferenc-csaky
Copy link

Purpose of the change

Inspired by apache/flink-connector-kafka#69, and apache/flink-connector-aws#121. Moves cassandra.py and test_cassandra.py from the core https://github.com/apache/flink repo to this one.

When running mvn clean install, downloads the testing infra scripts from the main Flink repo:

  • flink-python/dev/build-wheels.sh
  • flink-python/dev/install_command.sh
  • flink-python/dev/lint-python.sh

Verifying this change

The python tests can be run by the following commands:

mvn clean install -DskipTests
cd flink-python
chmod a+x dev/* 
./dev/lint-python.sh -e mypy,sphinx

Copy link

boring-cyborg bot commented Jan 29, 2024

Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html)

flink-python/setup.py Outdated Show resolved Hide resolved
@ferenc-csaky
Copy link
Author

@pvary @gaborgsomogyi JAR included, CI run passed.


os.makedirs(LIB_PATH)
connector_jar = \
glob.glob(CURRENT_DIR + '/target/test-dependencies/flink-connector-cassandra*.jar')[0]
Copy link

Choose a reason for hiding this comment

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

QQ: Is this an uber jar for the connector?

Copy link
Author

@ferenc-csaky ferenc-csaky Feb 6, 2024

Choose a reason for hiding this comment

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

Yes, it includes the necessary cassandra related deps:

<artifactId>maven-shade-plugin</artifactId>

Copy link

@pvary pvary left a comment

Choose a reason for hiding this comment

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

The packaging part looks good.
I have limited experience with the Cassandra connector, so I would like to have someone with Cassandra experience to review it too.

@ferenc-csaky
Copy link
Author

@echauchot Sorry to ping you out of the blue, I saw that you were an active contributor to the Cassandra connector. If you have some time, would you mind reviewing this PR? Thanks in advance!

@echauchot
Copy link
Contributor

@echauchot Sorry to ping you out of the blue, I saw that you were an active contributor to the Cassandra connector. If you have some time, would you mind reviewing this PR? Thanks in advance!

Hi @ferenc-csaky it would be with pleasure but I know nothing about python so I might not be the good person to review this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants