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

Guava Version Downgrade to 25.0-jre #7628

Merged

Conversation

kushagraThapar
Copy link
Member

  • Downgraded guava version to 25.0-jre
  • Copied Strings.lenientFormat() from guava library to azure cosmos internal Utils.java

@@ -552,4 +557,91 @@ public ValueHolder(V v) {
holder.v = dictionary.remove(key);
return holder.v != null;
}

Copy link
Member

Choose a reason for hiding this comment

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

Lets please get clarification on required claims context from OSS

Copy link
Member Author

@kushagraThapar kushagraThapar Jan 22, 2020

Choose a reason for hiding this comment

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

Waiting for PR to be approved, and then will start the OSS process.
Just want to make sure these changes are good and don't pose any hidden faults before starting the OSS process.

Copy link
Contributor

@moderakh moderakh left a comment

Choose a reason for hiding this comment

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

as RNTBD uses guava please talk to David making sure he is fine with downgrading guava.
also CELA approval for including guava code here.

@kushagraThapar
Copy link
Member Author

/azp run java - cosmos - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@g2vinay
Copy link
Member

g2vinay commented Jan 23, 2020

@JimSuplizio , @danieljurek , @mitchdenny
The checkout step is failing in Cosmos live test build.
Is it missing some required configuration ?
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=240921&view=results

@kushagraThapar
Copy link
Member Author

/azp run java - cosmos - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kushagraThapar
Copy link
Member Author

/azp run java - cosmos - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@JimSuplizio JimSuplizio left a comment

Choose a reason for hiding this comment

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

@kushagraThapar these changes aren't correct. You know that versions are not changed directly in the POM, they're either changes in the version_*.txt files in the case of built libraries or the external_dependencies.txt file in the case of external dependencies. The file you should have changed is external_dependencies.txt and then you should have run python eng/versioning/update_versions.py --ut external_dependency --sr from the root of the enlistment. This is the reason that the Analyze step is failing.

As for the checkout errors, if you were missing a configuration you would not have had ANY runs passing. @mitchdenny could this be because the pipelines are using v1 style git tasks (If you look at the last post on that thread it's someone else seeing a similar issue posted a few days ago).

@kushagraThapar
Copy link
Member Author

@kushagraThapar these changes aren't correct. You know that versions are not changed directly in the POM, they're either changes in the version_*.txt files in the case of built libraries or the external_dependencies.txt file in the case of external dependencies. The file you should have changed is external_dependencies.txt and then you should have run python eng/versioning/update_versions.py --ut external_dependency --sr from the root of the enlistment. This is the reason that the Analyze step is failing.

As for the checkout errors, if you were missing a configuration you would not have had ANY runs passing. @mitchdenny could this be because the pipelines are using v1 style git tasks (If you look at the last post on that thread it's someone else seeing a similar issue posted a few days ago).

@JimSuplizio Thank you Jim for correcting me here, I will update the external_dependencies.txt file and run the script and push my changes

Copy link
Member

@JimSuplizio JimSuplizio left a comment

Choose a reason for hiding this comment

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

From a versioning perspective, the changes to downgrade the guava version look correct.

@JimSuplizio
Copy link
Member

@JimSuplizio , @danieljurek , @mitchdenny
The checkout step is failing in Cosmos live test build.
Is it missing some required configuration ?
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=240921&view=results

Apparently when we're running these runs it's merging your changes on top of master to run. When you see the git exit code of 128 that means that your branch is more than 20 commits behind master. The correct course of action here would be to rebase master and push an updated PR.

@kushagraThapar
Copy link
Member Author

Waiting for CELA requirements to be fulfilled. Please don't merge this PR until then.

@moderakh
Copy link
Contributor

/azp run java - cosmos - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@kushagraThapar kushagraThapar merged commit 01e21d8 into Azure:master Jan 29, 2020
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.

6 participants