Conversation
Also pull pbjs and pbts from `npm bin` instead of global path.
…ut not when we just need the database name). The v1 protocol now enforces that /documents be included for root document paths.
4593dd2 to
52c7381
Compare
JustinBeckwith
left a comment
There was a problem hiding this comment.
LGTM w/ a few nits.
- This file shows up in the change list:
dev/protos/google/firestore/v1beta1/.DS_Store. Thinking that we want to make sure all.DS_Storefiles are ignored and removed. It's ok to do this in a follow up PR. - Are there any known breaking changes for consumers of the package? In the move to v1beta1, did we change/update any APIs in a breaking way? It would be really good to call these out in the description of the PR if so.
- Should we still be shipping the
dev/protos/google/firestore/v1beta1/directory at all?
|
@JustinBeckwith Thanks!
|
|
@crwilcox @jadekler Are we removing the |
|
@JustinBeckwith We decided not to. @mikelehen could you leave the v1beta1 gapic as-is, and just switch the manual to using v1? This helps users currently relying on the beta (because they remain unbroken for a little while longer), but mostly just makes the process a bit more seamless. |
|
I can undelete dev/src/v1beta1 (and dev/protos/google/firestore/v1beta/ too?) but I'm unclear what benefit this provides. We have to delete it before we go 1.x, right? And so if users depend on it directly, they'll need to stick with a 0.x version. |
jkwlui
left a comment
There was a problem hiding this comment.
blocking this until we bring back v1beta1 back in.
|
@JustinBeckwith Per discussions, I've updated this PR to keep the v1beta1 gapic client and expose both v1beta1 and v1 gapic clients to developers, but the manual SDK uses v1 internally. @kinwa91 The docs check you added in #511 is failing because of two broken links that came from the v1 protos. I see you added steps to synth.py to automatically fix them, but when I re-run synth.py I'm actually seeing it revert your changes to .kokoro/docs.sh. Am I doing something wrong? |
|
I went ahead and checked in just the proto changes necessary to make CI pass, but I had to revert some of the other changes synth.py made. |
|
@kinwa91 v1beta1 has been revived. Can you approve? Thanks! |
|
synthtool hasn't been released to reflect the changes to docs.sh. I will cut a release today. LGTM |
Update the nodejs-firestore client to use the v1 protos and endpoint. Mostly this is a no-op, though I did have to tweak the way we serialize empty document paths because the backend now requires
/project/[project]/database/[database]/documentsfor empty document paths instead of/project/[project]/database/[database]The v1beta1 gapic client is preserved for now and can still be used, but the manually-curated API uses v1.