-
Notifications
You must be signed in to change notification settings - Fork 31
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(flags): expose subscriptions api behind feature flag for test harness #110
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
- Coverage 67.16% 65.60% -1.56%
==========================================
Files 57 59 +2
Lines 5944 6086 +142
==========================================
+ Hits 3992 3993 +1
- Misses 1667 1807 +140
- Partials 285 286 +1 ☔ View full report in Codecov by Sentry. |
remove api |
@@ -162,6 +163,7 @@ var serveCmd = &cobra.Command{ | |||
|
|||
func init() { | |||
// api | |||
serveCmd.Flags().Var(&ExperimentalFeatures{}, "enable-feature", "enable features that are disabled by default since they are considered experimental") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serveCmd.Flags().Var(&ExperimentalFeatures{}, "enable-feature", "enable features that are disabled by default since they are considered experimental") | |
serveCmd.Flags().Var(&ExperimentalFeatures{}, "enable-feature", "enable experimental features that are disabled by default") |
@@ -226,3 +229,28 @@ func init() { | |||
serveCmd.Flags().SortFlags = false | |||
rootCmd.AddCommand(serveCmd) | |||
} | |||
|
|||
type ExperimentalFeatures []string |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this move to the flags.go
file?
@@ -39,6 +42,13 @@ func New(api api.API, config *Config) api.Subsystem { | |||
r.POST("/promises/:id/resolve", s.resolvePromise) | |||
r.POST("/promises/:id/reject", s.rejectPromise) | |||
|
|||
// Subscription API (experimental) | |||
if slices.Contains(config.Enable, "subscription") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if slices.Contains(config.Enable, "subscription") { | |
if slices.Contains(config.Enable, "subscriptions") { |
|
||
// Subscription | ||
|
||
type SearchSubscriptionsParams struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We probably won't have a search subscriptions API, just a list all subscriptions on a promise (which is also paginated),
} | ||
|
||
type CreateSubscriptionHeader struct { | ||
RequestId string `header:"request-id"` // qq: just for grpc? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an optional parameter for tracking requests cross-systems. If provided this field is used in corresponding logs, if not provided I believe we generate a uuid for each request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can set in both http/grpc
Fixes #109