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

chore: make version embedding more robust #451

Merged
merged 3 commits into from
Oct 16, 2020

Conversation

igorbernstein2
Copy link
Contributor

@igorbernstein2 igorbernstein2 commented Oct 7, 2020

The previous approach used the jar manifest. Unfortunately that approaches falls apart when the
consumer shades the client jar. The new approach uses a combination of releasetool's version tag
replacement and maven-resources-plugin to generate a class with an embedded version string.

@igorbernstein2 igorbernstein2 requested review from a team as code owners October 7, 2020 20:45
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Oct 7, 2020
@igorbernstein2 igorbernstein2 requested a review from kolea2 October 7, 2020 20:45
@codecov
Copy link

codecov bot commented Oct 7, 2020

Codecov Report

Merging #451 into master will increase coverage by 21.43%.
The diff coverage is 66.66%.

Impacted file tree graph

@@              Coverage Diff              @@
##             master     #451       +/-   ##
=============================================
+ Coverage     59.20%   80.63%   +21.43%     
- Complexity       19     1116     +1097     
=============================================
  Files             2      106      +104     
  Lines           125     6942     +6817     
  Branches         18      369      +351     
=============================================
+ Hits             74     5598     +5524     
- Misses           34     1145     +1111     
- Partials         17      199      +182     
Impacted Files Coverage Δ Complexity Δ
...n/templates/com/google/cloud/bigtable/Version.java 50.00% <50.00%> (ø) 1.00 <1.00> (?)
...ble/data/v2/stub/EnhancedBigtableStubSettings.java 96.91% <100.00%> (ø) 22.00 <0.00> (?)
...ud/bigtable/data/v2/stub/BigtableStubSettings.java 74.52% <0.00%> (ø) 7.00% <0.00%> (?%)
...table/data/v2/stub/ReadModifyWriteRowCallable.java 100.00% <0.00%> (ø) 3.00% <0.00%> (?%)
.../google/cloud/bigtable/data/v2/models/Filters.java 81.72% <0.00%> (ø) 16.00% <0.00%> (?%)
...gtable/admin/v2/BigtableInstanceAdminSettings.java 96.55% <0.00%> (ø) 6.00% <0.00%> (?%)
...igtable/gaxx/retrying/ApiResultRetryAlgorithm.java 15.38% <0.00%> (ø) 3.00% <0.00%> (?%)
...a/v2/stub/readrows/ReadRowsResumptionStrategy.java 95.45% <0.00%> (ø) 11.00% <0.00%> (?%)
...le/cloud/bigtable/data/v2/models/BulkMutation.java 97.56% <0.00%> (ø) 10.00% <0.00%> (?%)
...bigtable/data/v2/stub/metrics/CompositeTracer.java 100.00% <0.00%> (ø) 33.00% <0.00%> (?%)
... and 97 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 346a42d...cccd64c. Read the comment docs.

@igorbernstein2 igorbernstein2 changed the title chore: Make version embedding more robust chore: make version embedding more robust Oct 7, 2020
The previous approach used the jar manifest. Unfortunately that approaches falls apart when the
consumer shades the client jar. The new approach uses a combination of releasetool's version tag
replacement and maven-resources-plugin to generate a class with an embedded version string.
@product-auto-label product-auto-label bot added the api: bigtable Issues related to the googleapis/java-bigtable API. label Oct 8, 2020
kolea2
kolea2 previously approved these changes Oct 16, 2020
@kolea2 kolea2 dismissed their stale review October 16, 2020 17:45

new changes to be reviewed

// Note: this file MUST be named GoogleUtils.java so that it can be picked up by releasetool
public class GoogleUtils {
public static final String BIGTABLE_CLIENT_VERSION =
"1.16.1-SNAPSHOT"; // {x-version-update:google-cloud-bigtable:current}
Copy link
Contributor

Choose a reason for hiding this comment

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

@chingor13, to get this to work, I'm guessing we need to add a path here? https://github.com/googleapis/release-please/blob/master/src/releasers/java-yoshi.ts#L182 Any thing else?

@igorbernstein2
Copy link
Contributor Author

As discussed offline: This is a temporary solution. In the future we will stop relying on maven resource filtering and instead hardcode the version and bump it by release-please

@elharo
Copy link

elharo commented Oct 29, 2020

It's worth understanding why the customer was shading, and removing the need for that.

ad548 pushed a commit to ad548/java-bigtable that referenced this pull request Mar 13, 2021
* chore: make version embedding more robust

The previous approach used the jar manifest. Unfortunately that approaches falls apart when the
consumer shades the client jar. The new approach uses a combination of releasetool's version tag
replacement and maven-resources-plugin to generate a class with an embedded version string.

* Simplify by using a file allowlisted in release-tool

* Revert "Simplify by using a file allowlisted in release-tool"

This reverts commit 22d831c.
igorbernstein2 added a commit to igorbernstein2/java-bigtable that referenced this pull request Apr 9, 2021
Use new feature in releasetool (googleapis/releasetool#317) to manage the client version instead of maven tricks introduced in googleapis#451
igorbernstein2 added a commit that referenced this pull request Apr 13, 2021
* chore: improve embedded version handling

Use new feature in releasetool (googleapis/releasetool#317) to manage the client version instead of maven tricks introduced in #451

* migrate to new version scheme

* rename back to Version

* config release-please for version bumps

* make sure file doesnt get clobbered

* add a couple of tests

* fmt
kolea2 pushed a commit to kolea2/java-bigtable that referenced this pull request May 20, 2021
* chore: improve embedded version handling

Use new feature in releasetool (googleapis/releasetool#317) to manage the client version instead of maven tricks introduced in googleapis#451

* migrate to new version scheme

* rename back to Version

* config release-please for version bumps

* make sure file doesnt get clobbered

* add a couple of tests

* fmt

(cherry picked from commit 1af8925)
kolea2 added a commit that referenced this pull request May 21, 2021
* chore: improve embedded version handling (#715)

* chore: improve embedded version handling

Use new feature in releasetool (googleapis/releasetool#317) to manage the client version instead of maven tricks introduced in #451

* migrate to new version scheme

* rename back to Version

* config release-please for version bumps

* make sure file doesnt get clobbered

* add a couple of tests

* fmt

(cherry picked from commit 1af8925)

* update version

* fix test

* fix: add back in extraFiles and fix file path (#833)

(cherry picked from commit f914954)

* fix test

Co-authored-by: Igor Bernstein <igorbernstein@google.com>
@igorbernstein2 igorbernstein2 deleted the robust-version branch February 8, 2023 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigtable Issues related to the googleapis/java-bigtable API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants