| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
This is just rewriting googleapis#934 as a ADR [skip ci].
There was a problem hiding this comment.
LGTM.
Sorry, something went wrong.
There was a problem hiding this comment.
LGTM
Sorry, something went wrong.
There was a problem hiding this comment.
But wait, after chatting with @houglum my scenario wasn't called out during his discussion.
Scenario:
User requests metadata with fields=name for a Bucket resource. The resource has versioning enabled but the Bucket metadata will be set to false, confusing the new user that versioning is disabled.
This is an accidental use-case that a user can make them to shoot themselves in the foot.
We should discuss this more, I misunderstood the state of the conversation.
Sorry, something went wrong.
So this hypothetical user is sophisticated enough to know that they can ask the server to not returns some fields, yet they are confused when one of the values they explicitly filtered out does not match the value in the server? I do not see it. I think folks sophisticated enough to request a subset of the fields know that all other fields cannot possibly have meaningful values. And we should be optimizing for he common case, which I believe is no filtering, in that case have to treat each field as potentially optional is super awkward, while the filtering case is harder to deal with, but you are doing something special. In other words, we should be making normal things easy, and weird things possible. PS: Not that it matters, but versioning happens to be optional, but I believe you were making a broader point about any field that just gets a weird default when the server does not send it back. |
Sorry, something went wrong.
|
Apologies, my last comment as you mentioned was a generalized statement. Preface: I don't have any external developer feedback here, and it is more a thought. An existing code base that uses the GCS C++ client library was first implemented by an experienced developer with field selection and some time after a new developer on the team hits this issue when accessing an unselected field. The developer may incur hours of debugging the issue because the value isn't consistent with the GCS service. The developer learns a lot about GCS the hard way. The alternative to this scenario is the new developer attempts to use a value not selected and finds |
Sorry, something went wrong.
|
That is a better argument, thanks. To make life easier in your scenario though, every developer has to use the awkward optional field API. And then have to do this always, even if they never use field filtering. They are paying for what they don't use. And I think we both agree that most of the time folks are not going to use field filtering. I do not like this tradeoff, making most folks pay, most of the time, for something that happens rarely is just not ideal. And look to the other APIs: proto3 no longer has has_field() for primitive types and strings. That would make the GCS API inconsistent with all the other C++ APIs based on protos, where there is always a value (false or 0 but a value). |
Sorry, something went wrong.
|
Agreed, we are on the same page on the given overhead for use-cases that may not be encountered often that would impact all developers using the library. I don't like it either, but I'd like to be make sure we address this appropriately. With respect to proto3, GCS API is proto2 API maybe there's rationale to have optional in this case? It's not consistent with other libraries, but that's because it isn't a proto3 API. |
Sorry, something went wrong.
There was a problem hiding this comment.
One request and LGTM.
Sorry, something went wrong.
|
PTAL. |
Sorry, something went wrong.
There was a problem hiding this comment.
Thanks, LGTM
Sorry, something went wrong.
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This is just rewriting #934 as a ADR [skip ci].
This change is