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

Upgraded AWS SDK to 1.11.817 #100

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

Conversation

pbruski
Copy link

@pbruski pbruski commented Jul 10, 2020

No description provided.

@pbruski pbruski requested a review from a team as a code owner July 10, 2020 13:11
@@ -27,6 +27,7 @@ configurations.all {
"com.fasterxml.jackson.core:jackson-core" -> useVersion("2.9.4")
"org.slf4j:slf4j-api" -> useVersion("1.8.0-alpha2")
"org.apache.httpcomponents:httpcore" -> useVersion("4.4.9")
"commons-codec:commons-codec" -> useVersion("1.11")
Copy link
Contributor

Choose a reason for hiding this comment

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

A new dependency? What is it for?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, it's an old one. Just a version bump.

Copy link
Contributor

Choose a reason for hiding this comment

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

Old dependency, new version conflict. That's what happens if libs don't use version ranges: uncountable bogus version conflicts.

@dagguh dagguh requested a review from a team July 10, 2020 13:32
@pbruski pbruski force-pushed the Upgraded-AWS-SDK-to-1.11.817 branch 2 times, most recently from d257383 to fd683f0 Compare July 10, 2020 14:36
Copy link
Contributor

@mzyromski-atlassian mzyromski-atlassian left a comment

Choose a reason for hiding this comment

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

@pbruski, what's the rationale behind the bump? Also, could you please add a CHANGELOG entry?

Copy link
Author

@pbruski pbruski left a comment

Choose a reason for hiding this comment

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

@pbruski, what's the rationale behind the bump? Also, could you please add a CHANGELOG entry?

New instance types, sure.

architecture
}

return "ubuntu-focal-20.04-${imgArchitecture}-server-20200701"
Copy link
Author

Choose a reason for hiding this comment

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

The new SDK allows use of new architecture - arm64

Copy link
Author

Choose a reason for hiding this comment

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

Hmmm. I've just realised that uses of aws-infrastructure will be broken if they use SDK older than the one where they introduced DescribeInstanceTypes. How do we handle that? That means a new major release, right?

Copy link
Contributor

@dagguh dagguh Jul 15, 2020

Choose a reason for hiding this comment

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

That's the problem with hiding version conflicts by default (damn you Maven). We can express that aws-infra requires at least this version, but it could be ignored.
Gradle consumers are in the clear. They can failOnVersionConflict to detect it early. And they can use dependency locks to control when the bump (and conflict) happens.
I tried to depend on AWS SDK SemVer range, but Maven would download every single POM for every version in the range (hundreds). It increased Maven build times by tens of minutes on cold caches. Gradle did no such thing.
Our examples for vendors use Maven, because the Atlassian ecosystem is built upon it.

So in general, it's not a problem for Gradlers, and it might be a problem for Maveners.
Even the loose Gradle users (without dep locks and strict conflicts) would not be impacted, because Gradle would pick the higher version anyway. Maven will pick the closest dep in the tree.

By accident, jira-performance-tests depends on aws-infrastructure first and then on aws-resources, so aws-infra AWS SDK will win. I confirmed it via ./mvnw -pl reference-jira-app-performance-tests dependency:tree in go/jpt repo. If we ever need to take the control of this, jira-performance-test can resolve this conflict in its POM.

So for now, we can stick to the current POM contracts:

Adding a requirement of a major version of a dependency is breaking a contract. Dropping a requirement of a major version of a dependency is a new contract.

If we ever find a way to (any of the following):

  • get failOnVersionConflict for Maven
  • get dependency locks for Maven (this is actually very probable)
  • get Maven to stop downloading the entire dependency version range
  • avoid Maven as a build tool

... then we can rest easy with the current POM contracts.
A better POM contract could help us too, but I'm out of ideas there.

@pbruski pbruski force-pushed the Upgraded-AWS-SDK-to-1.11.817 branch from fd683f0 to 44ca2c8 Compare July 14, 2020 13:40
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