Skip to content

Another gogo improvement: inline stringmap structs to avoid another u… #64

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 14, 2017
Merged

Another gogo improvement: inline stringmap structs to avoid another u… #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7879566
Another gogo improvement: inline stringmap structs to avoid another u…
Mar 12, 2017
0a8dc51
Add support for returning attributes from mixer API calls.
Mar 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mixer/v1/attributes.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ message Attributes {
map<int32, google.protobuf.Timestamp> timestamp_attributes = 8 [(gogoproto.nullable) = false, (gogoproto.stdtime) = true];
map<int32, google.protobuf.Duration> duration_attributes = 9 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
map<int32, bytes> bytes_attributes = 10;
map<int32, StringMap> stringMap_attributes = 11;
map<int32, StringMap> stringMap_attributes = 11 [(gogoproto.nullable) = false];

// Attributes that should be removed from the specified attribute context. Deleting
// attributes which aren't currently in the attribute context is not considered an error.
Expand Down
7 changes: 5 additions & 2 deletions mixer/v1/check.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,12 @@ message CheckResponse {
// Index of the request this response is associated with
int64 request_index = 1;

// The attributes to use for this response
Attributes attribute_update = 2;

// Indicates whether or not the preconditions succeeded
google.rpc.Status result = 2 [(gogoproto.nullable) = false];
google.rpc.Status result = 3 [(gogoproto.nullable) = false];

Copy link
Contributor

Choose a reason for hiding this comment

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

"2" used to be "result", now it is used for attribute_update. This is bad. It breaks the backward compatibility. You never re-use a proto index.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, given the phase the project is in I didn't think backward compat was an issue yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

We did this (and in general have been changing field indices) because these protos aren't being persisted anywhere today - the field number change should have no effect on anyone. We're still pre-Alpha so all bets are off w.r.t. backwards compatibility. Post official Alpha release I totally agree this kind of shuffling of fields must never happen.

Copy link
Contributor

Choose a reason for hiding this comment

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

It is between proxy and mixer. Without this change, we can safely say all proxy versions works with all mixer versions. Now with this change, we could not say that any more.
The demo users have to be careful which version of proxy and mixer can work together. They are not always using the latest.

// The amount of time for which this result can be considered valid, given the same inputs
google.protobuf.Duration expiration = 3 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
google.protobuf.Duration expiration = 4 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
}
9 changes: 6 additions & 3 deletions mixer/v1/quota.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,15 @@ message QuotaResponse {
// Index of the request this response is associated with.
int64 request_index = 1;

// The attributes to use for this response
Attributes attribute_update = 2;

// Indicates whether the quota request was successfully processed.
google.rpc.Status result = 2 [(gogoproto.nullable) = false];
google.rpc.Status result = 3 [(gogoproto.nullable) = false];

// The amount of time the returned quota can be considered valid, this is 0 for non-expiring quotas.
google.protobuf.Duration expiration = 3 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];
google.protobuf.Duration expiration = 4 [(gogoproto.nullable) = false, (gogoproto.stdduration) = true];

// The total amount of quota returned, may be less than requested.
int64 amount = 4;
int64 amount = 5;
}
5 changes: 4 additions & 1 deletion mixer/v1/report.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ message ReportResponse {
// Index of the request this response is associated with
int64 request_index = 1;

// The attributes to use for this response
Attributes attribute_update = 2;

// Indicates whether the report was processed or not
google.rpc.Status result = 2 [(gogoproto.nullable) = false];
google.rpc.Status result = 3 [(gogoproto.nullable) = false];
}