feat: Cmd evaluation #2609

mexes20 · 2024-11-02T12:03:51Z

Description

evaluation command line for local testing

Closes issue #2560

Resolve #2560

Checklist

I have tested this code
I have added unit test to cover this code
I have updated the documentation (README.md and /website/docs)
I have followed the contributing guide

netlify · 2024-11-02T12:04:08Z

✅ Deploy Preview for go-feature-flag-doc-preview ready!

Name	Link
🔨 Latest commit	`9fd49d6`
🔍 Latest deploy log	https://app.netlify.com/sites/go-feature-flag-doc-preview/deploys/674084c3d2f53c000942ca7f
😎 Deploy Preview	https://deploy-preview-2609--go-feature-flag-doc-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

thomaspoignant

Hey @mexes20 thanks for your contribution, I am looking forward for this new cli.

I have made a first pass and found some things to fix before being able to have a working command line.
Can you try to fix it and ensure that the code compiles before re-requesting a review?

Thanks again for your contribution 🙏

thomaspoignant · 2024-11-04T20:49:56Z

cmd/cli/evaluate.go

+
+	"github.com/thomaspoignant/go-feature-flag/ffuser"
+	"github.com/thomaspoignant/go-feature-flag/retriever"
+	"github.com/thomaspoignant/go-feature-flag/variation"


issue: This package does not exist in go-feature-flag I am not sure why you are importing it? I see that you are trying to use it in your code too. Did you miss committing something or is this an error?

thomaspoignant · 2024-11-04T20:53:10Z

cmd/cli/evaluate.go

+	"io/ioutil"
+
+	"github.com/thomaspoignant/go-feature-flag/ffuser"
+	"github.com/thomaspoignant/go-feature-flag/retriever"


issue: This import is not used. We should remove it.

Suggested change

"github.com/thomaspoignant/go-feature-flag/retriever"

thomaspoignant · 2024-11-04T20:54:17Z

cmd/cli/evaluate.go

+}
+
+type FlagConfig struct {
+	Flags map[string]*variation.Flag `yaml:"flags"`


suggestion: As mention in a previous comment, the package variation does not exists, what you are looking at here is flag.Flag

Suggested change

Flags map[string]*variation.Flag `yaml:"flags"`

Flags map[string]*flag.Flag `yaml:"flags"`

thomaspoignant · 2024-11-04T20:55:20Z

cmd/cli/evaluate.go

+	evaluateCmd.Flags().StringVarP(&flagName, "flag", "f", "", "Name of the flag to evaluate")
+	evaluateCmd.Flags().StringVarP(&evaluationContext, "evaluation-context", "e", "{}", "Evaluation context in JSON format")
+
+	evaluateCmd.MarkFlagRequired("config")


suggestion: We should handle the error return by MarkFlagRequired to be sure that we are not letting the user pass without the required parameters.

thomaspoignant · 2024-11-04T20:57:43Z

cmd/cli/main.go

+		fmt.Fprintf(os.Stderr, "Error executing command: %v\n", err)
+		os.Exit(1)


suggestion: Since we want to exit in error and write an error I would suggest to use log.Fatalf instead.

Suggested change

fmt.Fprintf(os.Stderr, "Error executing command: %v\n", err)

os.Exit(1)

log.Fatalf("Error executing command: %v\n", err)

thomaspoignant · 2024-11-04T21:11:01Z

cmd/cli/evaluate.go

+	user := ffuser.NewUser(targetingKey)
+	for k, v := range context {
+		if k != "targetingKey" {
+			user.Custom[k] = v


issue: Custom is not a public attribute so you can't add things in the context like this.

suggestion: Use the function AddCustomAttribute(k, v) instead

Suggested change

user.Custom[k] = v

evalCtx.AddCustomAttribute(k, v)

thomaspoignant · 2024-11-04T21:11:48Z

cmd/cli/evaluate.go

+	return &config, nil
+}
+
+func parseContext(contextJSON string) (*ffuser.User, error) {


suggestion: ffuser.User is deprecated, please use ffcontext.EvaluationContext instead.

thomaspoignant · 2024-11-04T21:14:47Z

cmd/cli/evaluate.go

+type EvaluationResult struct {
+	TrackEvents   bool        `json:"trackEvents"`
+	VariationType string      `json:"variationType"`
+	Failed        bool        `json:"failed"`
+	Version       string      `json:"version"`
+	Reason        string      `json:"reason"`
+	ErrorCode     string      `json:"errorCode"`
+	Value         interface{} `json:"value"`
+	Cacheable     bool       `json:"cacheable"`
+}


question: Why do you need to create a new response struct here? You can probably use model.VariationResult instead that has the exact same signature (except that it is a generic type).

This struct is available here.

thomaspoignant · 2024-11-04T21:16:52Z

cmd/cli/evaluate.go

+	return nil
+}
+
+func evaluateSingleFlag(flag *variation.Flag, user *ffuser.User) (*EvaluationResult, error) {


Suggested change

func evaluateSingleFlag(flag *variation.Flag, user *ffuser.User) (*EvaluationResult, error) {

func evaluateSingleFlag(flag *variation.Flag, user *ffcontext.EvaluationContext) (*EvaluationResult, error) {

thomaspoignant · 2024-11-04T21:20:08Z

cmd/cli/evaluate.go

+func evaluateAllFlags(config *FlagConfig, user *ffuser.User) error {
+	results := make(map[string]*EvaluationResult)
+
+	for flagName, flag := range config.Flags {


issue: Here config.Flags will always be empty since you've passed nothing for the flag name here.

suggestion: You can directly use the function AllFlagsState that perform the evaluation for all the flags.

mexes20 · 2024-11-05T16:01:28Z

Hey @mexes20 thanks for your contribution, I am looking forward for this new cli.

I have made a first pass and found some things to fix before being able to have a working command line. Can you try to fix it and ensure that the code compiles before re-requesting a review?

Thanks again for your contribution 🙏

Okay ser, I'm working on it now.

mexes20 · 2024-11-15T10:31:20Z

@thomaspoignant I'm still working on this ser, just having a hard time with some parts but I'll fix it soon

Signed-off-by: Thomas Poignant <[email protected]>

codecov · 2024-11-21T17:55:29Z

Codecov Report

Attention: Patch coverage is 92.90780% with 10 lines in your changes missing coverage. Please review.

Project coverage is 84.73%. Comparing base (8c2e215) to head (9fd49d6).
Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/cli/main.go	69.23%	4 Missing ⚠️
cmd/cli/evaluate/evaluate.go	92.10%	2 Missing and 1 partial ⚠️
cmd/cli/evaluate/evaluate_cmd.go	91.66%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2609      +/-   ##
==========================================
+ Coverage   84.50%   84.73%   +0.22%     
==========================================
  Files         106      111       +5     
  Lines        5021     5162     +141     
==========================================
+ Hits         4243     4374     +131     
- Misses        616      624       +8     
- Partials      162      164       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

Flaky Tests Detection - Detect and resolve failed and flaky tests
JS Bundle Analysis - Avoid shipping oversized bundles

Signed-off-by: Thomas Poignant <[email protected]>

sonarqubecloud · 2024-11-22T13:19:37Z

Quality Gate failed

Failed conditions
6.2% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

mexes20 added 2 commits October 31, 2024 13:53

feat: command line evaluation for local testing

80a39c4

fixes

296ed02

github-actions bot assigned thomaspoignant Nov 2, 2024

github-actions bot requested a review from thomaspoignant November 2, 2024 12:04

mexes20 changed the title ~~Cmd evaluation~~ feat: Cmd evaluation Nov 3, 2024

mexes20 added 2 commits November 3, 2024 01:19

Merge branch 'main' into cmd-evaluation

bd198d5

Merge branch 'main' into cmd-evaluation

47625dd

thomaspoignant requested changes Nov 4, 2024

View reviewed changes

mexes20 and others added 3 commits November 10, 2024 11:38

Merge branch 'main' into cmd-evaluation

bfda922

fix: rework command line to use the existing code

a4fcb40

WIP

8ac82d9

chore: simplify cli

97c976b

Signed-off-by: Thomas Poignant <[email protected]>

thomaspoignant force-pushed the cmd-evaluation branch from 1242877 to 97c976b Compare November 21, 2024 17:51

Merge branch 'main' into cmd-evaluation

ba1b592

thomaspoignant added 2 commits November 22, 2024 09:28

Merge branch 'main' into cmd-evaluation

fe380b8

refactor to add test

862ab55

Signed-off-by: Thomas Poignant <[email protected]>

thomaspoignant force-pushed the cmd-evaluation branch from 49ac3f9 to 862ab55 Compare November 22, 2024 10:41

thomaspoignant added 3 commits November 22, 2024 12:20

Add build for cli in goreleaser

71c7f0b

Signed-off-by: Thomas Poignant <[email protected]>

Update documentation

6484b9f

Signed-off-by: Thomas Poignant <[email protected]>

Add deploy doc dockerhub

9fd49d6

Signed-off-by: Thomas Poignant <[email protected]>

thomaspoignant approved these changes Nov 22, 2024

View reviewed changes

thomaspoignant merged commit 1c9292d into thomaspoignant:main Nov 22, 2024
21 of 22 checks passed

BrewTestBot mentioned this pull request Nov 23, 2024

go-feature-flag-relay-proxy 1.39.0 Homebrew/homebrew-core#198738

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Cmd evaluation #2609

feat: Cmd evaluation #2609

mexes20 commented Nov 2, 2024

netlify bot commented Nov 2, 2024 •

edited

Loading

thomaspoignant left a comment

thomaspoignant Nov 4, 2024

thomaspoignant Nov 4, 2024

thomaspoignant Nov 4, 2024

thomaspoignant Nov 4, 2024

thomaspoignant Nov 4, 2024

thomaspoignant Nov 4, 2024

thomaspoignant Nov 4, 2024

thomaspoignant Nov 4, 2024

thomaspoignant Nov 4, 2024

thomaspoignant Nov 4, 2024

mexes20 commented Nov 5, 2024

mexes20 commented Nov 15, 2024

codecov bot commented Nov 21, 2024 •

edited

Loading

sonarqubecloud bot commented Nov 22, 2024

	Flags map[string]*variation.Flag `yaml:"flags"`
	Flags map[string]*flag.Flag `yaml:"flags"`

		fmt.Fprintf(os.Stderr, "Error executing command: %v\n", err)
		os.Exit(1)

	fmt.Fprintf(os.Stderr, "Error executing command: %v\n", err)
	os.Exit(1)
	log.Fatalf("Error executing command: %v\n", err)

	func evaluateSingleFlag(flag variation.Flag, user ffuser.User) (*EvaluationResult, error) {
	func evaluateSingleFlag(flag variation.Flag, user ffcontext.EvaluationContext) (*EvaluationResult, error) {

feat: Cmd evaluation #2609

feat: Cmd evaluation #2609

Conversation

mexes20 commented Nov 2, 2024

Description

Closes issue #2560

Checklist

netlify bot commented Nov 2, 2024 • edited Loading

✅ Deploy Preview for go-feature-flag-doc-preview ready!

thomaspoignant left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mexes20 commented Nov 5, 2024

mexes20 commented Nov 15, 2024

codecov bot commented Nov 21, 2024 • edited Loading

Codecov Report

sonarqubecloud bot commented Nov 22, 2024

Quality Gate failed

netlify bot commented Nov 2, 2024 •

edited

Loading

codecov bot commented Nov 21, 2024 •

edited

Loading