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.

Sign up for GitHub

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

Jump to bottom

Spark session client #70

Merged
merged 3 commits into from
Jun 15, 2023
Merged

Spark session client #70

merged 3 commits into from
Jun 15, 2023

Conversation

jcamachor
Copy link
Contributor

@jcamachor jcamachor commented Jun 13, 2023 •
edited
Loading

Relates to #17.

This patch contains multiple changes to enable testing using Spark session client, and in general, non-JDBC based connections. Concretely:

  • Subclassing of ConnectionManager into JDBCConnectionManager and SparkConnectionManager.
  • Moving configuration input parsing methods into their own utility class.
  • TelemetryRegistry can rely on any ConnectionManager.
  • Testing for Delta using session client (Hudi and Iceberg disabled for the time being, see Enable Hudi and Iceberg in Spark Session tests #71).

The PR includes moving to Java11, since Java17 causes some issues with Spark dependency: https://stackoverflow.com/questions/72724816/running-unit-tests-with-spark-3-3-0-on-java-17-fails-with-illegalaccesserror-cl

@jcamachor jcamachor force-pushed the spark_session_client branch 13 times, most recently from fcbb23a to 3230578 Compare June 14, 2023 04:51
@jcamachor jcamachor mentioned this pull request Jun 14, 2023
Open
@jcamachor jcamachor force-pushed the spark_session_client branch 4 times, most recently from ee3e018 to 17f6267 Compare June 14, 2023 05:45
@jcamachor
Initial drop. Contains:
95f1830 
- Subclassing of ConnectionManager into JDBCConnectionManager and SparkConnectionManager.
- Fix for logging dependency in pom.xml.
- Moving configuration input parsing methods into their own utility class.
- TelemetryRegistry can rely on any ConnectionManager.
@jcamachor
Move to Java11
1970329
@jcamachor
Copy link
Contributor Author

@ashvina , @anjagruenheid , could any of you review? Thanks

ashvina
ashvina approved these changes Jun 15, 2023
View reviewed changes
Copy link
Contributor

@ashvina ashvina left a comment

Choose a reason for hiding this comment

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

LGTM 👍

src/main/java/com/microsoft/lst_bench/Driver.java Outdated Show resolved Hide resolved
@jcamachor jcamachor merged commit cd25d04 into microsoft:main Jun 15, 2023
@jcamachor jcamachor deleted the spark_session_client branch June 15, 2023 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashvina ashvina ashvina approved these changes

@anjagruenheid anjagruenheid anjagruenheid approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jcamachor @ashvina @anjagruenheid