cody: experimental feature flag for .com#49920
Conversation
| userFlags, err := db.FeatureFlags().GetUserFlags(ctx, a.UID) | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
|
|
||
| return userFlags["cody-experimental"], nil |
There was a problem hiding this comment.
You should use featureflag.FromContext(ctx).GetBoolOr("cody-experimental", false) to get the correct result here, there cloud also be a global flag set :)
| if envvar.SourcegraphDotComMode() { | ||
| isEnabled, err := cody.IsCodyExperimentalFeatureFlagEnabled(ctx, r.db) |
There was a problem hiding this comment.
just for correctness of error messages, should we move this check after the next one, so the feature flag will be evaluated after the setting is checked?
Same for others.
| ctx, cancel := context.WithTimeout(r.Context(), maxRequestDuration) | ||
| defer cancel() |
There was a problem hiding this comment.
Just moved it up a couple of lines because now we need the ctx variable earlier.
| completionsConfig := conf.Get().Completions | ||
| if completionsConfig == nil || !completionsConfig.Enabled { | ||
| http.Error(w, "completions are not configured or disabled", http.StatusInternalServerError) |
There was a problem hiding this comment.
maybe the same about ordering here.
| defer cancel() | ||
|
|
||
| if envvar.SourcegraphDotComMode() { | ||
| isEnabled := cody.IsCodyExperimentalFeatureFlagEnabled(ctx) |
There was a problem hiding this comment.
just to be sure, did you check it works on this endpoint, too? I know we initialize the FF store on the resolver CTX, but not sure about HTTP handlers. (I hope we do that, but might want to check :))
There was a problem hiding this comment.
Yep, tried it locally, seems to work. We set the feature flag middleware here: https://sourcegraph.com/search?q=context%3Aglobal+repo%3A%5Egithub%5C.com%2Fsourcegraph%2Fsourcegraph%24+featureflag.Middleware&patternType=standard&sm=1&groupBy=path
Does that look ok?
## Test plan <!-- All pull requests REQUIRE a test plan: https://docs.sourcegraph.com/dev/background-information/testing_principles --> * Non-dotcom instances should work as before * If `cody-experimental` feature flag is not created, dotcom users will not be able to use completions endpoint and embeddings GQL API * If `cody-experimental` feature flag override is created for a user, that user should have access to completions and embeddings on dot com * Unauthorized users should not have access to either (cherry picked from commit 3c90dda)
This backports both - #49920 - #50668 to 5.0. The former is cherry picked to reduce merge conflicts and make potential future cherry picks a bit easier. I also had to backport adding a bazelignore entry for `client/cody` because I couldn't update the build files on this branch anymore. There seems to have been a few misses in buildfile updates so this PR is unfortunately quite noisy. ## Test plan CI and code review. --------- Co-authored-by: Rok Novosel <rok@sourcegraph.com> Co-authored-by: Beyang Liu <beyang@sourcegraph.com>
Test plan
cody-experimentalfeature flag is not created, dotcom users will not be able to use completions endpoint and embeddings GQL APIcody-experimentalfeature flag override is created for a user, that user should have access to completions and embeddings on dot com