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
DatastoreDb.Insert() sometimes returns null. #722
Comments
The test is certainly confusing and needs changing. The behaviour matches the underlying API, however - I would be somewhat reluctant to completely mask that, both for consistency with other platforms and because it may well be useful behaviour. (Aside from anything else, it would be odd for a user to read the API documentation and then get very different behaviour from the library.) @pcostell Do you have examples of where it's particularly useful to distinguish between allocated keys and client-provided keys? If we could add something into our library docs, that would be great. |
Ah, I hadn't noticed that the test is in your samples rather than in my repo :) I would take issue of your claim of "should" in the test, as the code is behaving exactly as documented. |
Ping for further input from @SurferJeffAtGoogle or @pcostell. (Marked as release blocking because changing it later would be a breaking change.) |
I believe this is just for reduced network traffic. There is no need for us to tell you about the keys you already know about. From the user point of view, there isn't really any difference between the two. Where it can be important is if you do some sort of:
You'll end up accidentally creating two entities, rather than 1. Most of our libraries have modified commit so that when you call |
Just reopening for now - the "most of our libraries" comment from Patrick sounds like it may be something we want to do... although of course it makes for interesting things around asynchrony. Jeffrey: I'm assuming your closing aspect was around the returning of the keys? Do you think you'd expect and/or like the entities to be mutated after commit? (It feels slightly surprising to me, but I can see it being handy.) If we're happy on the return front but want to explore the mutation side, I'll create a separate issue for that. |
Reclosing this, so a new issue can focus on whether to update key properties. |
Source-Link: googleapis/googleapis@950dd73 Source-Link: googleapis/googleapis-gen@57ace17 Copy-Tag: eyJwIjoiYXBpcy9Hb29nbGUuQ2xvdWQuQ29tcHV0ZS5WMS8uT3dsQm90LnlhbWwiLCJoIjoiNTdhY2UxNzhjODU0M2M0NDYyNzgyMzA0NTkxNzgyMGQ2N2I0OTBjMSJ9
Source-Link: googleapis/googleapis@950dd73 Source-Link: googleapis/googleapis-gen@57ace17 Copy-Tag: eyJwIjoiYXBpcy9Hb29nbGUuQ2xvdWQuQ29tcHV0ZS5WMS8uT3dsQm90LnlhbWwiLCJoIjoiNTdhY2UxNzhjODU0M2M0NDYyNzgyMzA0NTkxNzgyMGQ2N2I0OTBjMSJ9
Source-Link: googleapis/googleapis@950dd73 Source-Link: googleapis/googleapis-gen@57ace17 Copy-Tag: eyJwIjoiYXBpcy9Hb29nbGUuQ2xvdWQuQ29tcHV0ZS5WMS8uT3dsQm90LnlhbWwiLCJoIjoiNTdhY2UxNzhjODU0M2M0NDYyNzgyMzA0NTkxNzgyMGQ2N2I0OTBjMSJ9
Changes in this release: This is the first version of this package to depend on GAX v4. There are some breaking changes, both in GAX v4 and in the generated code. The changes that aren't specific to any given API are [described in the Google Cloud documentation](https://cloud.google.com/dotnet/docs/reference/help/breaking-gax4). We don't anticipate any changes to most customer code, but please [file a GitHub issue](https://github.com/googleapis/google-cloud-dotnet/issues/new/choose) if you run into problems. The most important change in this release is the use of the Grpc.Net.Client package for gRPC communication, instead of Grpc.Core. When using .NET Core 3.1 or .NET 5.0+ this should lead to a smaller installation footprint and greater compatibility (e.g. with Apple M1 chips). Any significant change in a core component comes with the risk of incompatibility, however - so again, please let us know if you encounter any issues. ### New features - Update Compute Engine API to revision 20220526 ([issue 722](googleapis#722)) ([commit 5d75c04](googleapis@5d75c04))
Changes in this release: This is the first version of this package to depend on GAX v4. There are some breaking changes, both in GAX v4 and in the generated code. The changes that aren't specific to any given API are [described in the Google Cloud documentation](https://cloud.google.com/dotnet/docs/reference/help/breaking-gax4). We don't anticipate any changes to most customer code, but please [file a GitHub issue](https://github.com/googleapis/google-cloud-dotnet/issues/new/choose) if you run into problems. The most important change in this release is the use of the Grpc.Net.Client package for gRPC communication, instead of Grpc.Core. When using .NET Core 3.1 or .NET 5.0+ this should lead to a smaller installation footprint and greater compatibility (e.g. with Apple M1 chips). Any significant change in a core component comes with the risk of incompatibility, however - so again, please let us know if you encounter any issues. ### New features - Update Compute Engine API to revision 20220526 ([issue 722](#722)) ([commit 5d75c04](5d75c04))
The documentation describes the behavior correctly:
https://googlecloudplatform.github.io/google-cloud-dotnet/docs/Google.Cloud.Datastore.V1/api/Google.Cloud.Datastore.V1.DatastoreDb.html#Google_Cloud_Datastore_V1_DatastoreDb_Insert_Google_Cloud_Datastore_V1_Entity_Google_Api_Gax_Grpc_CallSettings_
But I found this behavior confusing.
https://github.com/GoogleCloudPlatform/dotnet-docs-samples/blob/master/datastore/api/TaskListTest/DatastoreTest.cs#L223
Why not always return the key?
The text was updated successfully, but these errors were encountered: