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: ability to specify the Collection Group query scope in the V1 Admin API #762

Merged
merged 4 commits into from
Sep 30, 2019

Conversation

yoshi-automation
Copy link
Contributor

This PR was generated using Autosynth. 🌈

Here's the log from Synthtool:

synthtool > Executing /tmpfs/src/git/autosynth/working_repo/synth.py.
synthtool > Ensuring dependencies.
synthtool > Pulling artman image.
latest: Pulling from googleapis/artman
Digest: sha256:66ca01f27ef7dc50fbfb7743b67028115a6a8acf43b2d82f9fc826de008adac4
Status: Image is up to date for googleapis/artman:latest
synthtool > Cloning googleapis.
synthtool > Running generator for google/firestore/admin/artman_firestore_v1.yaml.
synthtool > Generated code into /home/kbuilder/.cache/synthtool/googleapis/artman-genfiles/js/firestore-admin-v1.
synthtool > Running generator for google/firestore/artman_firestore.yaml.
synthtool > Generated code into /home/kbuilder/.cache/synthtool/googleapis/artman-genfiles/js/firestore-v1beta1.
synthtool > Running generator for google/firestore/artman_firestore_v1.yaml.
synthtool > Generated code into /home/kbuilder/.cache/synthtool/googleapis/artman-genfiles/js/firestore-v1.
synthtool > Replaced '../../package.json' in dev/src/v1/firestore_admin_client.js.
synthtool > Replaced '../../package.json' in dev/src/v1beta1/firestore_client.js.
synthtool > Replaced '../../package.json' in dev/src/v1/firestore_client.js.
synthtool > Replaced 'return this\\._innerApiCalls\\.listen\\(options\\);' in dev/src/v1beta1/firestore_client.js.
synthtool > Replaced 'return this\\._innerApiCalls\\.listen\\(options\\);' in dev/src/v1/firestore_client.js.
.eslintignore
.eslintrc.yml
.github/ISSUE_TEMPLATE/bug_report.md
.github/ISSUE_TEMPLATE/feature_request.md
.github/ISSUE_TEMPLATE/support_request.md
.github/release-please.yml
.jsdoc.js
.kokoro/common.cfg
.kokoro/continuous/node10/common.cfg
.kokoro/continuous/node10/docs.cfg
.kokoro/continuous/node10/lint.cfg
.kokoro/continuous/node10/samples-test.cfg
.kokoro/continuous/node10/system-test.cfg
.kokoro/continuous/node10/test.cfg
.kokoro/continuous/node12/common.cfg
.kokoro/continuous/node12/test.cfg
.kokoro/continuous/node8/common.cfg
.kokoro/continuous/node8/test.cfg
.kokoro/docs.sh
.kokoro/lint.sh
.kokoro/presubmit/node10/common.cfg
.kokoro/presubmit/node10/docs.cfg
.kokoro/presubmit/node10/lint.cfg
.kokoro/presubmit/node10/samples-test.cfg
.kokoro/presubmit/node10/system-test.cfg
.kokoro/presubmit/node10/test.cfg
.kokoro/presubmit/node12/common.cfg
.kokoro/presubmit/node12/test.cfg
.kokoro/presubmit/node8/common.cfg
.kokoro/presubmit/node8/test.cfg
.kokoro/presubmit/windows/common.cfg
.kokoro/presubmit/windows/test.cfg
.kokoro/publish.sh
.kokoro/release/docs.cfg
.kokoro/release/docs.sh
.kokoro/release/publish.cfg
.kokoro/samples-test.sh
.kokoro/system-test.sh
.kokoro/test.bat
.kokoro/test.sh
.kokoro/trampoline.sh
.nycrc
.prettierignore
.prettierrc
CODE_OF_CONDUCT.md
CONTRIBUTING.md
LICENSE
README.md
codecov.yaml
renovate.json
samples/README.md
synthtool > Replaced 'https:\\/\\/cloud\\.google\\.com[\\s\\*]*http:\\/\\/(.*)[\\s\\*]*\\)' in dev/src/v1beta1/doc/google/protobuf/doc_timestamp.js.
synthtool > Replaced 'https:\\/\\/cloud\\.google\\.com[\\s\\*]*http:\\/\\/(.*)[\\s\\*]*\\)' in dev/src/v1/doc/google/protobuf/doc_timestamp.js.
synthtool > No replacements made in **/doc/google/protobuf/doc_timestamp.js for pattern toISOString\], maybe replacement is not longer needed?

> core-js@2.6.9 postinstall /tmpfs/src/git/autosynth/working_repo/node_modules/core-js
> node scripts/postinstall || echo "ignore"

Thank you for using core-js ( https://github.com/zloirock/core-js ) for polyfilling JavaScript standard library!

The project needs your help! Please consider supporting of core-js on Open Collective or Patreon: 
> https://opencollective.com/core-js 
> https://www.patreon.com/zloirock 

Also, the author of core-js ( https://github.com/zloirock ) is looking for a good job -)


> protobufjs@6.8.8 postinstall /tmpfs/src/git/autosynth/working_repo/node_modules/protobufjs
> node scripts/postinstall


> @google-cloud/firestore@2.3.0 prepare /tmpfs/src/git/autosynth/working_repo
> npm run compile


> @google-cloud/firestore@2.3.0 compile /tmpfs/src/git/autosynth/working_repo
> tsc -p . && cp -r dev/protos build && cp -r dev/test/fake-certificate.json build/test/fake-certificate.json && cp dev/src/v1beta1/*.json build/src/v1beta1/ && cp dev/src/v1/*.json build/src/v1/ && cp dev/conformance/test-definition.proto build/conformance && cp dev/conformance/test-suite.binproto build/conformance

npm notice created a lockfile as package-lock.json. You should commit this file.
added 611 packages from 898 contributors and audited 1835 packages in 27.375s
found 0 vulnerabilities


> @google-cloud/firestore@2.3.0 fix /tmpfs/src/git/autosynth/working_repo
> gts fix

installing semver@^5.5.0
installing minimist@^1.2.0
installing espree@^3.5.4
synthtool > Cleaned up 2 temporary directories.
synthtool > Wrote metadata to synth.metadata.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 21, 2019
@codecov
Copy link

codecov bot commented Sep 21, 2019

Codecov Report

Merging #762 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #762   +/-   ##
=======================================
  Coverage   96.46%   96.46%           
=======================================
  Files          20       20           
  Lines        2236     2236           
  Branches      467      467           
=======================================
  Hits         2157     2157           
  Misses         24       24           
  Partials       55       55

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 a425b8b...a14488d. Read the comment docs.

@bcoe bcoe changed the title [CHANGE ME] Re-generated to pick up changes in the API or client library generator. feat: new proto annotations, types, introduces firestore default host Sep 30, 2019
@bcoe
Copy link
Contributor

bcoe commented Sep 30, 2019

@alexander-fenster @schmidt-sebastian mind giving this a quick eyeball to make sure the description is reasonable? this seems to pull in quite a few things.

@schmidt-sebastian
Copy link
Contributor

I think the major change in this PR is that it adds index management for Collection Group indices. As far as I can tell, the rest of the changes should not affect the SDK's behavior. My stab at a PR description would be:

feat: ability to specify the Collection Group query scope in the V1 Admin API.

As a further comment, we already have https://github.com/googleapis/nodejs-firestore/blob/4cb19e9f4933bc8dbd77f4f9dc877b8050b2c09f/dev/protos/firestore_proto_api.d.ts (4141 lines versus 19543 lines in this PR) and https://github.com/googleapis/nodejs-firestore/blob/4cb19e9f4933bc8dbd77f4f9dc877b8050b2c09f/dev/protos/firestore_proto_api.js (7186 lines versus 48836 lines). If we can, it would be nice if we could only include one copy of these files and make sure that we only use the minimal sets of features that the library/the end user may need.

FWIW, we currently build with these settings:

"${PBJS}" --proto_path=. --js_out=import_style=commonjs,binary:library \

@alexander-fenster
Copy link
Contributor

@schmidt-sebastian @bcoe The .d.ts files for client libraries are currently not used (they will be used by micro-generators when they come), and I'm guessing that for Firestore, which does not expose GAPIC interface directly and has a lot of manually written TypeScript code, the generated .d.ts files in protos/ are not really needed.

Suggestion: let's unlink the generated .d.ts file in synth.py since they already have a better one available. If you both are OK with it I'll update synth.py in the same PR.

@schmidt-sebastian
Copy link
Contributor

@alexander-fenster We do expose the GAPIC client (see here).

That being said, since the files are unused, I am in favor of your suggestion to remove them from the PR. If we need to expose the files at a later point then we should look at how we can best reconcile the two versions.

@alexander-fenster
Copy link
Contributor

Updated this PR - please take a look!

@alexander-fenster alexander-fenster changed the title feat: new proto annotations, types, introduces firestore default host feat: ability to specify the Collection Group query scope in the V1 Admin API Sep 30, 2019
@alexander-fenster alexander-fenster merged commit b16cd40 into master Sep 30, 2019
@alexander-fenster alexander-fenster deleted the autosynth branch September 30, 2019 22:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

5 participants