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

[ci] add jdks to test matrix #1131

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

[ci] add jdks to test matrix #1131

wants to merge 14 commits into from

Conversation

marscher
Copy link
Member

@marscher marscher commented Apr 29, 2023

Added a mixture of JDK versions to our testing matrix. Version 8, 11, and 17 are being checked.

TODOs:

  • For some matrix elements the ivy dependencies are not installed (this leads to a failure in the test_sql*.py scripts)
  • Javadoc in JDK17 does not produce the same output as before.

@marscher marscher marked this pull request as ready for review April 29, 2023 11:46
@codecov
Copy link

codecov bot commented Apr 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7363ce8) 87.84% compared to head (f692203) 87.84%.
Report is 126 commits behind head on master.

❗ Current head f692203 differs from pull request most recent head e0e0ca9. Consider uploading reports for the commit e0e0ca9 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1131   +/-   ##
=======================================
  Coverage   87.84%   87.84%           
=======================================
  Files         112      112           
  Lines       10276    10276           
  Branches     4032     4032           
=======================================
  Hits         9027     9027           
  Misses        698      698           
  Partials      551      551           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@marscher marscher added this to the 1.5 milestone Apr 29, 2023
@marscher
Copy link
Member Author

@Thrameos I could use some help to figure out why the ivy deps are not found for some jobs - thank you :)

@Thrameos
Copy link
Contributor

Thrameos commented May 4, 2023

Sure. Though there may be other ways to skin that cat like calling it once and shipping the artifacts arounf. Been a while since I looked at it.

@Thrameos
Copy link
Contributor

Thrameos commented May 4, 2023

I looked through the logs. It is not an ivy failure. The first step of the process is to create the artifact deps which are then used for all builds. Thus if the failure was with ivy it would have failed in the first stage of the build and every build after it would have failed as well. The fact that some worked and some did not indicates that the dependencies are there but they didn't work for every build.

So lets look at the possibilities:

  1. The artifact failed to download and unpack properly. No evidence as the state says it was successful. Best way to debug is to add an "ls" or equivalent to download stage to see if everything was unpacked.
  2. The artifact unpacked but the jars did not work for the version of the JDK tested. This matches the behavior. (Though it took me looking at the patch as the JDK versions were random to the Python versions. We should add the JDK name to the description to make it clear.)

The jars being pulled are too new for 8 so most likely it is JDK 52.0 version conflict (causing one type of fail) and something has changed in 17 that is not allowing the driver to load in 17. The last one is more of a mystery as I rarely get things to fail forward unless they are hitting the ByteBuffer symbol conflict bug.

To diagnose I will have to install the same versions of JDK and the jars and try to get them to manually load in hopes of forcing an error. The JDK silently ignores any jars that fail to load from the classpath, so manual loading is required to get diagnostics. I will need to investigate further.

Safest fix for the JDK 8 is to add an option to skip the dbc tests for those builds. JDK is dead as a doornail at this point and it is hard to ensure that 3rd party jars will work with it going forward. 17 needs investigation.

@marscher
Copy link
Member Author

marscher commented May 4, 2023

thanks for looking into it. I'll be on vacation next week. So I wont be available that time. Do you intend to push the release in the next two weeks?

@Thrameos
Copy link
Contributor

Thrameos commented May 4, 2023

I was aiming for the end of April but I can push it back. We have a lot of things left to resolve so the time may be needed anyway.

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

2 participants