Streamline QuotaDescriptor. #35
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
louiscryan
reviewed
Jan 30, 2017
| PER_MINUTE_LIMIT = 2; | ||
| } | ||
| // The amount of time allocated quota remains valid before it is | ||
| // automatically released. If this is 0, then allocated quota is |
There was a problem hiding this comment.
Are we happy using 0 as opposed to a wrapped value?
|
By wrapped value, do you mean -1? If so, what do you think would be the
benefit?
…On Mon, Jan 30, 2017 at 12:31 PM, Louis Ryan ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In mixer/v1/config/descriptor/quota_descriptor.proto
<#35 (review)>:
> - // scaling, whereas imprecise quotas can support a nearly unbounded
- // level of scaling at the cost of potential accounting inaccuracies.
- bool precise = 8;
-
- enum QuotaKind {
- // Quota values are always explicitly manipulated via API calls.
- ALLOCATION_LIMIT = 0;
-
- // Quota limit expresses a maximum amount over a rolling time interval
- PER_SECOND_LIMIT = 1;
-
- // Quota limit expresses a maximum amount over a rolling time interval
- PER_MINUTE_LIMIT = 2;
- }
+ // The amount of time allocated quota remains valid before it is
+ // automatically released. If this is 0, then allocated quota is
Are we happy using 0 as opposed to a wrapped value?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#35 (review)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AVucHZup0ihe9lGCPAmk-IqbtzBiwio4ks5rXkiugaJpZM4LwzVK>
.
|
|
Nope I mean
https://github.com/google/protobuf/blob/master/src/google/protobuf/wrappers.proto#L83
i.e. nullable
On Mon, Jan 30, 2017 at 4:50 PM, Martin Taillefer <[email protected]>
wrote:
… By wrapped value, do you mean -1? If so, what do you think would be the
benefit?
On Mon, Jan 30, 2017 at 12:31 PM, Louis Ryan ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
> ------------------------------
>
> In mixer/v1/config/descriptor/quota_descriptor.proto
> <#35 (review)>:
>
> > - // scaling, whereas imprecise quotas can support a nearly unbounded
> - // level of scaling at the cost of potential accounting inaccuracies.
> - bool precise = 8;
> -
> - enum QuotaKind {
> - // Quota values are always explicitly manipulated via API calls.
> - ALLOCATION_LIMIT = 0;
> -
> - // Quota limit expresses a maximum amount over a rolling time interval
> - PER_SECOND_LIMIT = 1;
> -
> - // Quota limit expresses a maximum amount over a rolling time interval
> - PER_MINUTE_LIMIT = 2;
> - }
> + // The amount of time allocated quota remains valid before it is
> + // automatically released. If this is 0, then allocated quota is
>
> Are we happy using 0 as opposed to a wrapped value?
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#35 (review)>, or
mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AVucHZup0ihe9lGCPAmk-
IqbtzBiwio4ks5rXkiugaJpZM4LwzVK>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#35 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AIoKPJXS2w-jpGZ4rEvCtptDTxhtxrKaks5rXoVCgaJpZM4LwzVK>
.
|
|
I don't see the benefit. An expiration window of 0 is semantically invalid
so it makes a fine sentinel value. If there's a style guide recommendation
to use a wrapper, I can certainly change it. But on the face of it, I think
0 is fine.
On Mon, Jan 30, 2017 at 5:04 PM, Louis Ryan <[email protected]>
wrote:
… Nope I mean
https://github.com/google/protobuf/blob/master/src/
google/protobuf/wrappers.proto#L83
i.e. nullable
On Mon, Jan 30, 2017 at 4:50 PM, Martin Taillefer <
***@***.***>
wrote:
> By wrapped value, do you mean -1? If so, what do you think would be the
> benefit?
>
> On Mon, Jan 30, 2017 at 12:31 PM, Louis Ryan ***@***.***>
> wrote:
>
> > ***@***.**** commented on this pull request.
> > ------------------------------
> >
> > In mixer/v1/config/descriptor/quota_descriptor.proto
> > <#35 (review)>:
> >
> > > - // scaling, whereas imprecise quotas can support a nearly unbounded
> > - // level of scaling at the cost of potential accounting inaccuracies.
> > - bool precise = 8;
> > -
> > - enum QuotaKind {
> > - // Quota values are always explicitly manipulated via API calls.
> > - ALLOCATION_LIMIT = 0;
> > -
> > - // Quota limit expresses a maximum amount over a rolling time
interval
> > - PER_SECOND_LIMIT = 1;
> > -
> > - // Quota limit expresses a maximum amount over a rolling time
interval
> > - PER_MINUTE_LIMIT = 2;
> > - }
> > + // The amount of time allocated quota remains valid before it is
> > + // automatically released. If this is 0, then allocated quota is
> >
> > Are we happy using 0 as opposed to a wrapped value?
> >
> > —
> > You are receiving this because you authored the thread.
> > Reply to this email directly, view it on GitHub
> > <#35 (review)>, or
> mute
> > the thread
> > <https://github.com/notifications/unsubscribe-
auth/AVucHZup0ihe9lGCPAmk-
> IqbtzBiwio4ks5rXkiugaJpZM4LwzVK>
> > .
> >
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <#35 (comment)>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AIoKPJXS2w-
jpGZ4rEvCtptDTxhtxrKaks5rXoVCgaJpZM4LwzVK>
> .
>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#35 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AVucHbJAXOehGo6vHf_jJkaRfYczAMq7ks5rXoi3gaJpZM4LwzVK>
.
|
incfly
pushed a commit
to incfly/api
that referenced
this pull request
Jun 13, 2018
* Add golang echo backend sample Add sample Go backend server so that nodejs version and dependency can be removed. Go is likely already present on build system due to other istio components usage. * Remove nodejs echo backend
incfly
pushed a commit
to incfly/api
that referenced
this pull request
Jun 13, 2018
nacx
pushed a commit
to nacx/api
that referenced
this pull request
Apr 15, 2020
* Service Inventory gRPC server integration - server main should to utilize cobra command-line - Dockerfile to include "--server-name" param - minor gRPC server fix: NewTracer could return nil io.Closer - Jaeger ReporterConfig post endpoint should be CollectorEndpoint (instead of LocalAgentHostPort?) * First-cut pilot xDS interface routines for obtaining service inventory from Istio Pilot. Includes the following: - Routine to Connect to pilot - Send CDS/EDS streaming requests - Receive and handle DiscoveryResponse responses - Unmarshal clusters, and endpoints and print them - Test routine for above steps * Unmarshal CDS/EDS into Tetrate Service proto compatible - Separate out unmarshal CDS & EDS streams; each of those return tetrate.Service protos to be used to populate DB - Stitch Cluster/Endpoint fields into appropriate Service/Endpoint fields * chaned pkg name and a fix in jaegar.go * Service Inventory gRPC server integration - server main should to utilize cobra command-line - Dockerfile to include "--server-name" param - minor gRPC server fix: NewTracer could return nil io.Closer - Jaeger ReporterConfig post endpoint should be CollectorEndpoint (instead of LocalAgentHostPort?) * minor changes to fix errors * xDS CDS/EDS inventory with following further changes: - Renamed pkg/client to pkg/xdsclient; package renamed to xdsclient - Updated PLSQL file to update/insert services if not unique (occurs 2 times) - cmd/test-data/main.go now includes psql.CreateService() for populating DB services, clusters, endpoints, etc. - Minor fixes, compilation issues * first cut of xdsclient * Service Inventory/plsql change accommodating the following: - Delete list of services; plsql routines and sql routines for same - Makefile phony to include protos (force descriptor generation everytime) * Fixed compilation issues in cmd/test-data * Soft-delete service entries - Mark service deleted using Service.Status field enum - Update PLSQL delete_services() macro to update status field * fixes after testing.. * Further updates to xDS client and DB interfacing: - Integrate xdsclient code to server - Invoke DB updates with appropriate context - Added CreateServices([]Services) * re-arranged calling Unmarshall, test function change * First-cut pilot xDS interface routines for obtaining service inventory from Istio Pilot. Includes the following: - Routine to Connect to pilot - Send CDS/EDS streaming requests - Receive and handle DiscoveryResponse responses - Unmarshal clusters, and endpoints and print them - Test routine for above steps * Unmarshal CDS/EDS into Tetrate Service proto compatible - Separate out unmarshal CDS & EDS streams; each of those return tetrate.Service protos to be used to populate DB - Stitch Cluster/Endpoint fields into appropriate Service/Endpoint fields * chaned pkg name and a fix in jaegar.go * minor changes to fix errors * xDS CDS/EDS inventory with following further changes: - Renamed pkg/client to pkg/xdsclient; package renamed to xdsclient - Updated PLSQL file to update/insert services if not unique (occurs 2 times) - cmd/test-data/main.go now includes psql.CreateService() for populating DB services, clusters, endpoints, etc. - Minor fixes, compilation issues * first cut of xdsclient * Service Inventory/plsql change accommodating the following: - Delete list of services; plsql routines and sql routines for same - Makefile phony to include protos (force descriptor generation everytime) * Fixed compilation issues in cmd/test-data * fixes after testing.. * Further updates to xDS client and DB interfacing: - Integrate xdsclient code to server - Invoke DB updates with appropriate context - Added CreateServices([]Services) * re-arranged calling Unmarshall, test function change * Service/cluster DB updates incorporating the following: - Added cluster.name constraint for uniqueness required for ON CONFLICT updates - PLSQL delete_services() macro to delete all services, if no services string provided - Invoke delete prior to EDS updates * Service/cluster DB updates incorporating the following: - Added cluster.name constraint for uniqueness required for ON CONFLICT updates - delete_services() DB macro to delete all services, if no services string provided - Invoke delete prior to EDS updates * Minor Updates to Service Inventory xDS interfaces - Renamed pkg/xdsclient/xds_resp_unmarshall.go to xds_unmarshal.go - enum types for CDS/EDS type URLs - Removed unnecessary logs/prints in responses * Multiple Istio Pilot interfaces (change istio#1): Bring in grpc_server changes with appropriate command-line to inventory-server; Updated interface to use array of pilot addresses * Added :authority to work with VirtualService * Handling array of pilots * Addressed PR Comments, Moved xdsclient_test.go to different dir * Service Inventory gRPC server integration (istio#35) * Service Inventory gRPC server integration - server main should to utilize cobra command-line - Dockerfile to include "--server-name" param - minor gRPC server fix: NewTracer could return nil io.Closer - Jaeger ReporterConfig post endpoint should be CollectorEndpoint (instead of LocalAgentHostPort?) * Moving "--server-name" flags to deployment from Dockerfile. Earlier commits (4c45233) are still valid. * Azure onboarding improvements (istio#50) * Automatically onboard all subscriptions * Better look for user emails * Unified Makefiles * Added onboarding APIs (istio#49) * Added onboarding APIs * Fetch service principals when fetching users * API method to bind services to policies (groups) * Basic import users and group implementation * Better comments in proto files * Added filtering capabilities to the onboarding interfaces * Service Inventory with cached updates: - Handling of multiple pilot addresses - Common cached/incremental update framework - cached DB updates to DB; Update services to do add/mod/del DB updates * Minor fix to Gopkg.lock to use correct prometheus client_golang revision * mTLS changes; UT changes * removed certs and fixed errors in yaml * Minor changes to cache infra - Cache tests for add/mod/delete, list and delta updates - Cache to use keyFunc instead of service-specific function; moving service key to plsql pkg * Minor fixes to unit test, and earlier PR review comments; Use map to gather list of locality zones sent to service * mtls fixes and templatization * minor fix * UT fix * changes to xdshandler to take a map * Service Category improvement fixes addressing the following: - Added cloud provider based on zone/locality - header inclusions, and updated variables (post rebase) - Gopkg fixes * changed intermediary cert validity to 365 days - PR Comment * Service Catalog PR fixes, and minor improvements: - Addressed PR fixes except cache, SQL library for queries (will cover next) - Ensure atleast one endpoint in identifying provider - Using pkg/logs instead - Use third_party/protobuf for proto compilation - Added inventory.proto-descriptor to .gitignore * Minor updates to Service Inventory in branch to enable correct compilation packages; Updated tetrate.sql with (missed) merge changes from master * move xdsclient_test.go to /test - PR comment * Service Inventory improvement fixes addressing the following: - Cache wrapper on top of istio cache supporting custom key/equal/deltas; moved cache to pkg/cache - plsql/service.go now initializes a service-specific cache instance for local package operations - Moved out service-related routines into plsql/service.go - Replaced PLSQL macros calls with sql prepare tx context across insert/queries - PLSQL tetrate.sql tables with correct primary keys/constraints - fixed deployment yaml missing mount/secret stanzas (rebase merge issue) - Addressed all other PR comments * Ridding of istio constraints in Gopkg.toml; also CircleCI build fails resolving to 1.0.0 * Service Catalog improvements incorporating the following: - Addressed all PR comments across packages - Included bunch relevant unit tests - Used errors.Wrap for annotated errors with stack trace, pkg/log, and t.log * Addressing PR Comments by zack * Fixed minor compilation issues post rebase to master * Added UT tests for cache expiry/eviction, and cloud providers * Minor compilation issues fixed post rebase to master * Incorporated recent PR comments including the following: - Endpoint now handles IPv4/v6 addresses - Moved Service (ListServices), and Service binding routines into plsql/service.go - Added log context to pkg/cache - api/inventory/v1/Makefile won't force rebuilding of proto sources (make CircleCI happy) - Included log/errors/debug specific to package as appropriate - Fixes to golinter, imports, formatting & statement recommendations Mirrored from https://github.com/tetrateio/tetrate @ 38397984da9b30a32eac5cb6fca59adba80ce578
nacx
pushed a commit
to nacx/api
that referenced
this pull request
Feb 23, 2022
* use generic message instead of protobuf.Struct Signed-off-by: Shriram Rajagopalan <[email protected]> * undos Signed-off-by: Shriram Rajagopalan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This generalizes refresh windows and eliminates the API level distinction between allocation and rate quotas.
Allocated quota now just has an expiration period which determines when how long an allocation 'survives' until it is automatically freed. If you specify a expiration time of 0, then the quota is never freed.
This removes the notion of precise/imprecise quotas. That was just a leftover of how things are handled in GCP and isn't really an appropriate top-level concern.