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: Support For Proto Column in Spanner #1829

Conversation

gauravpurohit06
Copy link
Contributor

PR Includes

  • Implementation for Proto Column & Enum which includes
    • Serialisation & Deserialisation Logic
    • Support to pass proto descriptor while creating database
  • Samples
    • DDL
    • DQL & DML
    • Mutation & Read
  • Unit Tests

@gauravpurohit06 gauravpurohit06 requested review from a team as code owners March 28, 2023 09:41
@product-auto-label product-auto-label bot added the size: xl Pull request size is extra large. label Mar 28, 2023
@snippet-bot
Copy link

snippet-bot bot commented Mar 28, 2023

Here is the summary of changes.

You are about to add 6 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/nodejs-spanner API. label Mar 28, 2023
@gauravpurohit06 gauravpurohit06 changed the title Support For Proto Column in Spanner feat: Support For Proto Column in Spanner Mar 31, 2023
src/codec.ts Outdated Show resolved Hide resolved
src/codec.ts Outdated Show resolved Hide resolved
src/codec.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/codec.ts Outdated Show resolved Hide resolved
src/codec.ts Outdated Show resolved Hide resolved
src/codec.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/partial-result-stream.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
samples/system-test/spanner.test.js Outdated Show resolved Hide resolved
samples/database-create-with-proto-descriptor.js Outdated Show resolved Hide resolved
samples/database-create-with-proto-descriptor.js Outdated Show resolved Hide resolved
samples/database-update-with-proto-descriptor.js Outdated Show resolved Hide resolved
samples/database-update-with-proto-descriptor.js Outdated Show resolved Hide resolved
samples/database-update-with-proto-descriptor.js Outdated Show resolved Hide resolved
src/codec.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/partial-result-stream.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@skuruppu skuruppu left a comment

Choose a reason for hiding this comment

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

I only ended up reviewing the src/ files for now. Once we discuss the comments, I'm happy to look into tests and samples.

src/codec.ts Outdated Show resolved Hide resolved
src/codec.ts Outdated Show resolved Hide resolved
src/codec.ts Outdated
).finish();
} else {
throw new GoogleError(`protoMessageParams cannot be used for constructing ProtoMessage.
Please pass serializedBytes as value or message object with messageFunction`);
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know what serializedBytes is. Is it a type?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By serializedBytes I meant serialized buffer of the proto message. Thank you for flagging it out, updating the exception message.
protoMessageParams cannot be used to construct the ProtoMessage. Pass the serialized buffer of the proto-message as the value or provide the message object along with the corresponding messageFunction generated by protobufjs-cli.

src/codec.ts Outdated Show resolved Hide resolved
src/codec.ts Outdated Show resolved Hide resolved
src/partial-result-stream.ts Outdated Show resolved Hide resolved
src/partial-result-stream.ts Outdated Show resolved Hide resolved
src/partial-result-stream.ts Outdated Show resolved Hide resolved
src/partial-result-stream.ts Outdated Show resolved Hide resolved
src/transaction.ts Outdated Show resolved Hide resolved
@asthamohta asthamohta added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label May 18, 2023
samples/database-create-with-proto-descriptor.js Outdated Show resolved Hide resolved
samples/database-update-with-proto-descriptor.js Outdated Show resolved Hide resolved
samples/proto-column-dml-insert.js Outdated Show resolved Hide resolved
samples/proto-column-query.js Outdated Show resolved Hide resolved
samples/proto-column-read.js Outdated Show resolved Hide resolved
src/partial-result-stream.ts Show resolved Hide resolved
src/partial-result-stream.ts Outdated Show resolved Hide resolved
src/transaction.ts Outdated Show resolved Hide resolved
@surbhigarg92 surbhigarg92 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 7, 2023
@surbhigarg92 surbhigarg92 merged commit 1051c66 into googleapis:proto-column-enhancement-alpha Aug 7, 2023
harshachinta added a commit that referenced this pull request May 15, 2024
* feat: Support For Proto Column in Spanner (#1829)

* feat: Generated Proto Changes

Changes need to be reverted.

* feat: Implementation for Proto Message & Enum

-Adding Logic for Serialization & Deserialization
-New Type Codes and utilities

* feat: Proto static files and typings and generated descriptors

* sample: Adding DML, DQL, DML, table insert & read samples.

* style: Lint

* test: Adding unit tests

* refactor: minor refactoring

* refactor: minor refactoring

* test: Adding integration tests

* docs: Adding docs

* test: Adding sample Integration Tests

* refactor: Minor refactoring and updating comments/docs.

* test: Making test fixes

* refactor: Minor refactoring and lint fixes

* refactor: Minor refactoring and lint fixes

* Updating docs and minor changes.

* test: fixing test

* refactor: minor changes

* refactor: minor refactoring

* docs: Updating docs & comments

* feat(spanner): fix lint

* fix(spanner: lint

* fix(spanner): lint

* fix(spanner): lint

* fix(spanner): lint

* feat(spanner): fix db name

* feat(spanner): remove it.only

* feat(spanner): fix tests lint

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* feat(spanner): remove samples and sample tests

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* feat(spanner): update schema to avoid reserved keyword

* feat(spanner): add samples and sample tests

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

* feat(spanner): code refactoring

---------

Co-authored-by: Gaurav Purohit <[email protected]>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/nodejs-spanner API. size: xl Pull request size is extra large.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants