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

feat: Add bundle proto building #271

Merged
merged 10 commits into from Sep 18, 2020
Merged

feat: Add bundle proto building #271

merged 10 commits into from Sep 18, 2020

Conversation

wu-hui
Copy link
Contributor

@wu-hui wu-hui commented Jun 24, 2020

  • Introduces a shell script to setup a local googleapis repo, and patch bundle.proto and its build file to the repo, and run synthtool with that repo.

  • As a result, bundle.proto under the Node.js sdk will the removed, and the update.sh script will clone from this repo.

  • Commit Add bundle.proto. has the manual changes, while the other commits has the results of running the new shell script.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 24, 2020
@wu-hui wu-hui requested a review from BenWhitehead June 24, 2020 15:45
@wu-hui wu-hui added the api: firestore Issues related to the googleapis/java-firestore API. label Jun 24, 2020
@BenWhitehead BenWhitehead added this to the v2 milestone Jun 24, 2020
@BenWhitehead
Copy link
Collaborator

Due to dependency on proto generation, this can't merge until master is ready for v2 commits.

@BenWhitehead
Copy link
Collaborator

@wu-hui I've not resolved the merge issues blocking synthool PRs being able to merge to master. Everything on master should be up-to-date. Can you please rebase this PR in prep for review?

@BenWhitehead BenWhitehead removed this from the v2 milestone Aug 11, 2020
# Conflicts:
#	.kokoro/dependencies.sh
#	.kokoro/nightly/samples.cfg
#	.kokoro/presubmit/samples.cfg
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1/FirestoreClient.java
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1/FirestoreSettings.java
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1/stub/FirestoreStub.java
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1/stub/FirestoreStubSettings.java
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1/stub/GrpcFirestoreStub.java
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1beta1/FirestoreClient.java
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1beta1/FirestoreSettings.java
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1beta1/package-info.java
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1beta1/stub/FirestoreStub.java
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1beta1/stub/FirestoreStubSettings.java
#	google-cloud-firestore/src/main/java/com/google/cloud/firestore/v1beta1/stub/GrpcFirestoreStub.java
#	google-cloud-firestore/src/test/java/com/google/cloud/firestore/v1/FirestoreClientTest.java
#	google-cloud-firestore/src/test/java/com/google/cloud/firestore/v1beta1/FirestoreClientTest.java
#	google-cloud-firestore/src/test/java/com/google/cloud/firestore/v1beta1/MockFirestoreImpl.java
#	proto-google-cloud-firestore-v1/src/main/java/com/google/firestore/v1/QueryProto.java
#	proto-google-cloud-firestore-v1/src/main/java/com/google/firestore/v1/StructuredQuery.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/ArrayValue.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/BatchGetDocumentsRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/BatchGetDocumentsResponse.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/BeginTransactionRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/BeginTransactionResponse.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/CommitRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/CommitResponse.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/CommonProto.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/CreateDocumentRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/Cursor.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/DeleteDocumentRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/Document.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/DocumentChange.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/DocumentDelete.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/DocumentProto.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/DocumentRemove.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/DocumentTransform.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/ExistenceFilter.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/FirestoreProto.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/GetDocumentRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/ListCollectionIdsRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/ListCollectionIdsResponse.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/ListDocumentsRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/ListDocumentsResponse.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/ListenRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/ListenResponse.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/MapValue.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/Precondition.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/QueryProto.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/RollbackRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/RunQueryRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/RunQueryResponse.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/StructuredQuery.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/Target.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/TargetChange.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/TransactionOptions.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/UpdateDocumentRequest.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/Value.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/Write.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/WriteProto.java
#	proto-google-cloud-firestore-v1beta1/src/main/java/com/google/firestore/v1beta1/WriteResult.java
#	synth.metadata
#	synth.py
@codecov
Copy link

codecov bot commented Aug 24, 2020

Codecov Report

Merging #271 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #271      +/-   ##
============================================
- Coverage     72.51%   72.49%   -0.02%     
+ Complexity     1066     1065       -1     
============================================
  Files            64       64              
  Lines          5589     5589              
  Branches        690      690              
============================================
- Hits           4053     4052       -1     
  Misses         1320     1320              
- Partials        216      217       +1     
Impacted Files Coverage Δ Complexity Δ
...in/java/com/google/cloud/firestore/BulkWriter.java 71.02% <0.00%> (-0.47%) 28.00% <0.00%> (-1.00%)

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 3303e92...7cd7805. Read the comment docs.

@wu-hui
Copy link
Contributor Author

wu-hui commented Aug 24, 2020

@wu-hui I've not resolved the merge issues blocking synthool PRs being able to merge to master. Everything on master should be up-to-date. Can you please rebase this PR in prep for review?

Sorry for the delay!

It is now merged with master, please review:

b35c1df has all the manual changes.
5151105 has the generated code.

@wu-hui
Copy link
Contributor Author

wu-hui commented Sep 2, 2020

@BenWhitehead

I reverted changes (Query.java and StructuredQuery.java) unrelated to bundles.

update.sh Outdated Show resolved Hide resolved
synth.metadata Outdated Show resolved Hide resolved

syntax = "proto3";

package firestore;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thoughts on changing this to google.firestore.bundle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it does not matter because this proto is not meant to be used by other protos. Looking at the namespaces/packages generated from this, they don't have bundle in the package path. I think keeping it this way would be fine?

proto-google-cloud-firestore-bundle-v1/pom.xml Outdated Show resolved Hide resolved
@BenWhitehead
Copy link
Collaborator

This branch will need to be rebased/merged with master before the builds will be able to run against it again.

@wu-hui
Copy link
Contributor Author

wu-hui commented Sep 15, 2020

This branch will need to be rebased/merged with master before the builds will be able to run against it again.

Done.

@BenWhitehead
Copy link
Collaborator

This looks okay to merge for me now.

@schmidt-sebastian Can you give this a once-over incase I missed anything?

@@ -0,0 +1,47 @@
#!/bin/bash
Copy link
Contributor

Choose a reason for hiding this comment

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

Who runs this script? It might make sense to run this as part of synth.py rather than invoke synthtool from here. That would remove a manual step.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed offline, the proto will be moved to third_party after all, so everything should be automatic.

Can we first merge it for now, and move it to third_party in a followup PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@BenWhitehead

FYI, we will move the proto to third_party of googleapis after all, looks like the benefits are really attractive..

Copy link
Collaborator

Choose a reason for hiding this comment

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

Syncing of thrid_party to https://github.com/googleapis/googleapis is usually the following night, and autosynth should run later that night.

So I guess it's up to you on the urgency.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me do it in a followup PR.

@wu-hui wu-hui merged commit 994835c into master Sep 18, 2020
@wu-hui wu-hui deleted the wuandy/BundleProto branch September 18, 2020 14:10
gcf-merge-on-green bot pushed a commit that referenced this pull request Jan 20, 2021
🤖 I have created a release \*beep\* \*boop\* 
---
## [2.2.0](https://www.github.com/googleapis/java-firestore/compare/v2.1.0...v2.2.0) (2021-01-20)


### Features

* Add bundle proto building ([#271](https://www.github.com/googleapis/java-firestore/issues/271)) ([994835c](https://www.github.com/googleapis/java-firestore/commit/994835c0a3be077404afa60abd4d4685d17fb533))
* add bundle.proto from googleapis/googleapis ([#407](https://www.github.com/googleapis/java-firestore/issues/407)) ([37da386](https://www.github.com/googleapis/java-firestore/commit/37da386875d1b65121e8a9a92b1a000537f07625))
* add CollectionGroup#getPartitions(long) ([#478](https://www.github.com/googleapis/java-firestore/issues/478)) ([bab064e](https://www.github.com/googleapis/java-firestore/commit/bab064edde26325bf0041ffe28d4c63b97a089c5))
* add implicit ordering for startAt(DocumentReference) calls ([#417](https://www.github.com/googleapis/java-firestore/issues/417)) ([aae6dc9](https://www.github.com/googleapis/java-firestore/commit/aae6dc960f7c42830ceed23c65acaad3e457dcff))
* add max/min throttling options to BulkWriterOptions ([#400](https://www.github.com/googleapis/java-firestore/issues/400)) ([27a9397](https://www.github.com/googleapis/java-firestore/commit/27a9397f67e151d723241c80ccb2ec9f1bfbba1c))
* add success and error callbacks to BulkWriter ([#483](https://www.github.com/googleapis/java-firestore/issues/483)) ([3c05037](https://www.github.com/googleapis/java-firestore/commit/3c05037e8fce8d3ce4907fde85bd254fc98ea588))
* Implementation of Firestore Bundle Builder ([#293](https://www.github.com/googleapis/java-firestore/issues/293)) ([fd5ef90](https://www.github.com/googleapis/java-firestore/commit/fd5ef90b6681cc67aeee6c95f3de80267798dcd0))
* Release bundles ([#466](https://www.github.com/googleapis/java-firestore/issues/466)) ([3af065e](https://www.github.com/googleapis/java-firestore/commit/3af065e32b193931c805b576f410ad90124b43a7))


### Bug Fixes

* add @BetaApi, make BulkWriter public, and refactor Executor ([#497](https://www.github.com/googleapis/java-firestore/issues/497)) ([27ff9f6](https://www.github.com/googleapis/java-firestore/commit/27ff9f6dfa92cac9119d2014c24a0759baa44fb7))
* **build:** sample checkstyle violations ([#457](https://www.github.com/googleapis/java-firestore/issues/457)) ([777ecab](https://www.github.com/googleapis/java-firestore/commit/777ecabd1ce12cbc5f4169de6c23a90f98deac06))
* bulkWriter: writing to the same doc doesn't create a new batch ([#394](https://www.github.com/googleapis/java-firestore/issues/394)) ([259ece8](https://www.github.com/googleapis/java-firestore/commit/259ece8511db71ea79cc1a080eb785a15db88756))
* empty commit to trigger release-please ([fcef0d3](https://www.github.com/googleapis/java-firestore/commit/fcef0d302cd0a9339d82db73152289d6f9f67ff2))
* make BulkWriterOptions public ([#502](https://www.github.com/googleapis/java-firestore/issues/502)) ([6ea05be](https://www.github.com/googleapis/java-firestore/commit/6ea05beb3f27337bef910ca64f0e3f32de6b7e98))
* retry Query streams ([#426](https://www.github.com/googleapis/java-firestore/issues/426)) ([3513cd3](https://www.github.com/googleapis/java-firestore/commit/3513cd39ff43d26c8432c05ce20693350539ae8f))
* retry transactions that fail with expired transaction IDs ([#447](https://www.github.com/googleapis/java-firestore/issues/447)) ([5905438](https://www.github.com/googleapis/java-firestore/commit/5905438af6501353e978210808834a26947aae95))
* verify partition count before invoking GetPartition RPC ([#418](https://www.github.com/googleapis/java-firestore/issues/418)) ([2054ae9](https://www.github.com/googleapis/java-firestore/commit/2054ae971083277e1cf81c2b57500c40a6faa0ef))


### Documentation

* **sample:** normalize firestore sample's region tags ([#453](https://www.github.com/googleapis/java-firestore/issues/453)) ([b529245](https://www.github.com/googleapis/java-firestore/commit/b529245c75f770e1b47ca5d9561bab55a7610677))


### Dependencies

* remove explicit version for jackson ([#479](https://www.github.com/googleapis/java-firestore/issues/479)) ([e2aecfe](https://www.github.com/googleapis/java-firestore/commit/e2aecfec51465b8fb3413337a76f9a3de57b8500))
* update dependency com.google.cloud:google-cloud-conformance-tests to v0.0.12 ([#367](https://www.github.com/googleapis/java-firestore/issues/367)) ([2bdd846](https://www.github.com/googleapis/java-firestore/commit/2bdd84693bbd968cafabd0e7ee56d1a9a7dc31ca))
* update dependency com.google.cloud:google-cloud-conformance-tests to v0.0.13 ([#411](https://www.github.com/googleapis/java-firestore/issues/411)) ([e6157b5](https://www.github.com/googleapis/java-firestore/commit/e6157b5cb532e0184125355b12115058e72afa67))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.10.0 ([#383](https://www.github.com/googleapis/java-firestore/issues/383)) ([cb39ee8](https://www.github.com/googleapis/java-firestore/commit/cb39ee820c2f67e22da623f5a6eaa7ee6bf351e2))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.10.2 ([#403](https://www.github.com/googleapis/java-firestore/issues/403)) ([991dd81](https://www.github.com/googleapis/java-firestore/commit/991dd810360e654fca0b53e0611da0cd77febc7c))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.12.1 ([#425](https://www.github.com/googleapis/java-firestore/issues/425)) ([b897ffa](https://www.github.com/googleapis/java-firestore/commit/b897ffa90427a8f597c02c24f80d1d162be48b23))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.13.0 ([#430](https://www.github.com/googleapis/java-firestore/issues/430)) ([0f8f218](https://www.github.com/googleapis/java-firestore/commit/0f8f218678c3ddebb73748c382cab8e38c2f140d))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.14.1 ([#446](https://www.github.com/googleapis/java-firestore/issues/446)) ([e241f8e](https://www.github.com/googleapis/java-firestore/commit/e241f8ebbfdf202f00424177c69962311b37fc88))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.15.0 ([#460](https://www.github.com/googleapis/java-firestore/issues/460)) ([b82fc35](https://www.github.com/googleapis/java-firestore/commit/b82fc3561d1a397438829ab69df24141481369a2))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.16.0 ([#481](https://www.github.com/googleapis/java-firestore/issues/481)) ([ae98824](https://www.github.com/googleapis/java-firestore/commit/ae988245e6d6391c85414e9b6f7ae1b8148c3a6d))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.16.1 ([4ace93c](https://www.github.com/googleapis/java-firestore/commit/4ace93c7be580a8f7870e71cad2dc19bb5fdef29))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.17.0 ([#487](https://www.github.com/googleapis/java-firestore/issues/487)) ([e11e472](https://www.github.com/googleapis/java-firestore/commit/e11e4723bc75727086bee0436492f458def29ff5))
* update dependency com.google.cloud:google-cloud-shared-dependencies to v0.18.0 ([#495](https://www.github.com/googleapis/java-firestore/issues/495)) ([f78720a](https://www.github.com/googleapis/java-firestore/commit/f78720a155f1294321f05266b9a546bbf2cb9a04))
* update jackson dependencies to v2.11.3 ([#396](https://www.github.com/googleapis/java-firestore/issues/396)) ([2e176e2](https://www.github.com/googleapis/java-firestore/commit/2e176e2f864262f31e6f93705fa7e794023b9649))
---


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: firestore Issues related to the googleapis/java-firestore API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants