Skip to content

com.spotify:dns 3.2.2 which uses dnsjava 3.0.2#171

Merged
protocol7 merged 4 commits into
spotify:masterfrom
klaraward:patch-1
Sep 22, 2021
Merged

com.spotify:dns 3.2.2 which uses dnsjava 3.0.2#171
protocol7 merged 4 commits into
spotify:masterfrom
klaraward:patch-1

Conversation

@klaraward
Copy link
Copy Markdown
Contributor

See internal PR for some context - https://ghe.spotify.net/java/bom/pull/538

Comment thread folsom/pom.xml Outdated
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<artifacftId>junit</artifactId>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This doesn't look correct

Comment thread folsom/pom.xml Outdated
@protocol7
Copy link
Copy Markdown
Contributor

@klaraward seems like com.spotify:dns:3.2.2 was built with Java 11, while Folsom requires 8. Could we build com.spotify:dns with 8 as well?

@klaraward
Copy link
Copy Markdown
Contributor Author

@fdfzcq ?

@fdfzcq
Copy link
Copy Markdown
Contributor

fdfzcq commented Sep 20, 2021

@klaraward @protocol7 I've changed maven plugin version from 9 to 8, will it do? spotify/dns-java#47

@protocol7
Copy link
Copy Markdown
Contributor

@fdfzcq yes, I think that would address this issue. Would you plan to release an update for com.spotify:dns with this fix?

@fdfzcq
Copy link
Copy Markdown
Contributor

fdfzcq commented Sep 20, 2021

@protocol7 yes we plan to merge and release that PR today

@fdfzcq
Copy link
Copy Markdown
Contributor

fdfzcq commented Sep 20, 2021

@protocol7 we merged the PR just to realize jdk 8 is not compatible with the latest spotify/dns. We will switch it back to jdk 9 for now and can look into moving it to jdk 8 (or multiple versions) later.

@protocol7
Copy link
Copy Markdown
Contributor

@fdfzcq Thanks for the update.

@fdfzcq
Copy link
Copy Markdown
Contributor

fdfzcq commented Sep 21, 2021

@klaraward @protocol7 we have now released newest version compiled with jdk 8: https://github.com/spotify/dns-java/releases/tag/v3.3.1

Comment thread folsom/pom.xml Outdated
@protocol7
Copy link
Copy Markdown
Contributor

@fdfzcq spotify/dns-java#47 is a non-backwards compatible change in that it adds a new method to DnsSrvResolver. I don't know how common it would be to implement this among users of dns-java, but I could imagine it would be somewhat frequently implemented in tests. We can easily fix this in Folsom, but I would worry about rolling it, e.g. across the Spotify fleet.

@fdfzcq
Copy link
Copy Markdown
Contributor

fdfzcq commented Sep 21, 2021

@protocol7 right, erik actually pointed out it which I thought I fixed but didn't https://github.com/spotify/dns-java/pull/47/files#r711963085 We would like to eventually move all users to use resolveAsync but for now this shouldn't break them.

@fdfzcq
Copy link
Copy Markdown
Contributor

fdfzcq commented Sep 21, 2021

Comment thread folsom/pom.xml Outdated
@protocol7
Copy link
Copy Markdown
Contributor

@fdfzcq that did indeed do the trick, thanks!

@protocol7 protocol7 merged commit 0f434c0 into spotify:master Sep 22, 2021
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.

4 participants