- Notifications
You must be signed in to change notification settings - Fork904
chore: use Java 21 for building pgjdbc by default#3612
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I think we should be good to go for Java 21 for spawning the build. Looks like the CI succeeds, so we are good to go. |
LGTM. For anybody else reading this, we're going from 17 to 21 (which like 17 is an LTS release) for the build itself. Testing etc still happens on multiple JDK versions including 8 and none of this changes any of that. |
18fa7bd tof68a77dCompareappveyor.yml Outdated
| -psql -U postgres -c "CREATE LANGUAGE plpgsql" test | ||
| -psql -U postgres -c "CREATE EXTENSION hstore" test | ||
| -psql -U postgres -c "CREATE EXTENSION sslinfo" postgres |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Why do we have to change anything for this when we're only updating the JDK version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Oh wait, is this because appveyor was completely broken before and now it might actually work?
interestingly, ChatGPT figured out that |
That's awesome. Looks like it's mostly working too. I see some errors for the auth test. Feels like this appveyor stuff has been broken forever that I don't think whatever we did to the GHA environment to work was ever done there too (the explicit stuff in HBA to test the different auth types). If it's going to be annoying to get it to work, maybe just disable those tests on appveyor. |
There are still two failures, and I don't understand why the tests expect certain output values: Here we fail to authenticate. Did we get the user right? Should it be pre-created when creating the DB? Should the DB be preconfigured somehow? I'm a bit lost here. We get "The authentication type should match ==> expected: <MD5_PASSWORD> but was: ", however, it is hard to understand why exactly something should be MD5. |
by default the password_encryption will be set to scram-sha-256. I'd vote for dumping the md5 test Dave |
f28858e to743dac2Compare| # Customize pg_hba.conf | ||
| local pg_hba="/home/certdir/pg_hba.conf" | ||
| sed -i's/127.0.0.1\/32/0.0.0.0\/0/g'"${pg_hba}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I'm not sure what was the reason to have127.0.0.1/32 in the sourcepg_hba.conf file and replace it with0.0.0.0/0 later, so I replaced the value with the basepg_hba.conf file so we don't need to replace them. At the same time, I replaced0.0.0.0/0 withall to support bothipv4 andipv6 ips.
453e9ff toeb72cc3CompareI commend your persistence here |
Even though I managed to get both |
Note: the resulting binaries still target Java 8, so the change should notbe use-visible.However, we would use a newer toolset which should result in fewer bugs inthe generated binaries.wal_keep_segments is deprecated since PostgreSQL 13, so remove it from appveyor.yaml
4d273f1 intopgjdbc:masterUh oh!
There was an error while loading.Please reload this page.
Good work! |
Note: the resulting binaries still target Java 8, so the change should not be use-visible.
However, we would use a newer toolset which should result in fewer bugs in the generated binaries.