Repository: flipt-io/flipt-client-sdks (language: python)

Issue:
[FLI-1269] Ensure consistency between SDKs
I think we should audit our existing client SDKs in the various languages and ensure they have a similar API before we make any of them 1.0.

Heres a few areas I think we could check for consistency/improve upon:

1. Remove the `Result` types as public facing return types. Similar to how we now do in the new dart SDK ([https://github.com/flipt-io/flipt-client-sdks/pull/351#pullrequestreview-2287892390](https://github.com/flipt-io/flipt-client-sdks/pull/351#pullrequestreview-2287892390)), just return the underlying types instead of the wrapped `Result` types
2. Ensure consistent naming of Request/Response types. ie: `EvaluationRequest`, `EvaluationResponse`, `BooleanEvaluationResponse`, `VariantEvaluationResponse`, etc.
3. Ensure the methods are named consistently across SDKs (ie: `evaluateVariant` / `evaluate_variant` depending on language idioms, `evaluateBoolean`, `evaluateBatch`, `listFlags`)
4. Ensure all methods are supported in all SDKs. I think we may be missing `listFlags` in a few
5. Ensure clients are named similarly, ex: `flipt-client` instead of `flipt-client-{language}` for their published names. As each ecosystem will have its own package manager, so the `-{language}` bit is unnecessary for the public facing package name (where applicable, ie we still need `flipt-client` and `flipt-client-browser` as they are both NPM packages)
6. Ensure types/classes named consistently, ie: `FliptEvaluationClient` instead of `FliptClient` or maybe vice-versa?
7. Dont expose anything publicly in the API that references `Engine`, ie `EngineOpts/Options`. The fact that we call the Rust evaluator an engine under the hood should not leak externally
8. Use `optional` types for languages that support it. ie: `variantKey` and `variantAttachment` should be optional in Variant evaluation responses

Anything else? Feel free to add

<sub>From [SyncLinear.com](https://synclinear.com) | [FLI-1269](https://linear.app/flipt/issue/FLI-1269/ensure-consistency-between-sdks)</sub>

## SDKS to Refactor

- [x] Node
- [x] Browser
- [x] Go
- [x] Dart
- [x] Ruby
- [x] Python
- [x] Java

## Published

- [x] Node
- [x] Browser
- [ ] Go
- [x] Dart
- [ ] Ruby
- [ ] Python
- [x] Java


Discussion / hints:

Fix patch:
diff --git a/flipt-client-go/README.md b/flipt-client-go/README.md
index 377848c9..9e2c8695 100644
--- a/flipt-client-go/README.md
+++ b/flipt-client-go/README.md
@@ -23,7 +23,6 @@ This SDK currently supports the following OSes/architectures:
 - MacOS x86_64
 - MacOS arm64
 
-
 ### Glibc vs Musl
 
 Most Linux distributions use [Glibc](https://en.wikipedia.org/wiki/Glibc), but some distributions like Alpine Linux use [Musl](https://en.wikipedia.org/wiki/Musl). If you are using Alpine Linux, you will need to install the `musl` tagged version of the client.
@@ -52,7 +51,7 @@ import (
 )
 
 func main() {
-  evaluationClient, err := flipt.NewClient()
+  evaluationClient, err := flipt.NewEvaluationClient()
   if err != nil {
     log.Fatal(err)
   }
@@ -67,7 +66,7 @@ func main() {
     log.Fatal(err)
   }
 
-  fmt.Println(*variantResult.Result)
+  fmt.Println(*variantResult)
 }
 ```
 
diff --git a/flipt-client-go/evaluation.go b/flipt-client-go/evaluation.go
index 697fa42d..51c2265c 100644
--- a/flipt-client-go/evaluation.go
+++ b/flipt-client-go/evaluation.go
@@ -20,8 +20,8 @@ import (
 
 const statusSuccess = "success"
 
-// Client wraps the functionality of making variant and boolean evaluation of Flipt feature flags.
-type Client struct {
+// EvaluationClient wraps the functionality of making variant and boolean evaluation of Flipt feature flags.
+type EvaluationClient struct {
 	engine         unsafe.Pointer
 	namespace      string
 	url            string
@@ -30,9 +30,9 @@ type Client struct {
 	updateInterval int
 }
 
-// NewClient constructs a Client.
-func NewClient(opts ...clientOption) (*Client, error) {
-	client := &Client{
+// NewEvaluationClient constructs a Client.
+func NewEvaluationClient(opts ...clientOption) (*EvaluationClient, error) {
+	client := &EvaluationClient{
 		namespace: "default",
 	}
 
@@ -64,32 +64,32 @@ func NewClient(opts ...clientOption) (*Client, error) {
 }
 
 // clientOption adds additional configuration for Client parameters
-type clientOption func(*Client)
+type clientOption func(*EvaluationClient)
 
 // WithNamespace allows for specifying which namespace the clients wants to make evaluations from.
 func WithNamespace(namespace string) clientOption {
-	return func(c *Client) {
+	return func(c *EvaluationClient) {
 		c.namespace = namespace
 	}
 }
 
 // WithURL allows for configuring the URL of an upstream Flipt instance to fetch feature data.
 func WithURL(url string) clientOption {
-	return func(c *Client) {
+	return func(c *EvaluationClient) {
 		c.url = url
 	}
 }
 
 // WithUpdateInterval allows for specifying how often flag state data should be fetched from an upstream Flipt instance.
 func WithUpdateInterval(updateInterval int) clientOption {
-	return func(c *Client) {
+	return func(c *EvaluationClient) {
 		c.updateInterval = updateInterval
 	}
 }
 
 // WithClientTokenAuthentication allows authenticating with Flipt using a static client token.
 func WithClientTokenAuthentication(token string) clientOption {
-	return func(c *Client) {
+	return func(c *EvaluationClient) {
 		c.authentication = clientTokenAuthentication{
 			Token: token,
 		}
@@ -98,7 +98,7 @@ func WithClientTokenAuthentication(token string) clientOption {
 
 // WithJWTAuthentication allows authenticating with Flipt using a JSON Web Token.
 func WithJWTAuthentication(token string) clientOption {
-	return func(c *Client) {
+	return func(c *EvaluationClient) {
 		c.authentication = jwtAuthentication{
 			Token: token,
 		}
@@ -106,7 +106,7 @@ func WithJWTAuthentication(token string) clientOption {
 }
 
 // EvaluateVariant performs evaluation for a variant flag.
-func (e *Client) EvaluateVariant(_ context.Context, flagKey, entityID string, evalContext map[string]string) (*VariantResult, error) {
+func (e *EvaluationClient) EvaluateVariant(_ context.Context, flagKey, entityID string, evalContext map[string]string) (*VariantEvaluationResponse, error) {
 	ereq, err := json.Marshal(evaluationRequest{
 		NamespaceKey: e.namespace,
 		FlagKey:      flagKey,
@@ -131,15 +131,15 @@ func (e *Client) EvaluateVariant(_ context.Context, flagKey, entityID string, ev
 		return nil, err
 	}
 
-	if vr.Status == statusSuccess {
-		return vr, nil
+	if vr.Status != statusSuccess {
+		return nil, errors.New(vr.ErrorMessage)
 	}
 
-	return nil, errors.New(vr.ErrorMessage)
+	return vr.Result, nil
 }
 
 // EvaluateBoolean performs evaluation for a boolean flag.
-func (e *Client) EvaluateBoolean(_ context.Context, flagKey, entityID string, evalContext map[string]string) (*BooleanResult, error) {
+func (e *EvaluationClient) EvaluateBoolean(_ context.Context, flagKey, entityID string, evalContext map[string]string) (*BooleanEvaluationResponse, error) {
 	ereq, err := json.Marshal(evaluationRequest{
 		NamespaceKey: e.namespace,
 		FlagKey:      flagKey,
@@ -164,15 +164,15 @@ func (e *Client) EvaluateBoolean(_ context.Context, flagKey, entityID string, ev
 		return nil, err
 	}
 
-	if br.Status == statusSuccess {
-		return br, nil
+	if br.Status != statusSuccess {
+		return nil, errors.New(br.ErrorMessage)
 	}
+	return br.Result, nil
 
-	return nil, errors.New(br.ErrorMessage)
 }
 
 // EvaluateBatch performs evaluation for a batch of flags.
-func (e *Client) EvaluateBatch(_ context.Context, requests []*EvaluationRequest) (*BatchResult, error) {
+func (e *EvaluationClient) EvaluateBatch(_ context.Context, requests []*EvaluationRequest) (*BatchEvaluationResponse, error) {
 	evaluationRequests := make([]*evaluationRequest, 0, len(requests))
 
 	for _, ir := range requests {
@@ -203,14 +203,14 @@ func (e *Client) EvaluateBatch(_ context.Context, requests []*EvaluationRequest)
 		return nil, err
 	}
 
-	if br.Status == statusSuccess {
-		return br, nil
+	if br.Status != statusSuccess {
+		return nil, errors.New(br.ErrorMessage)
 	}
 
-	return nil, errors.New(br.ErrorMessage)
+	return br.Result, nil
 }
 
-func (e *Client) ListFlags(_ context.Context) (*ListFlagsResult, error) {
+func (e *EvaluationClient) ListFlags(_ context.Context) ([]Flag, error) {
 	flags := C.list_flags(e.engine)
 	defer C.destroy_string(flags)
 
@@ -222,15 +222,15 @@ func (e *Client) ListFlags(_ context.Context) (*ListFlagsResult, error) {
 		return nil, err
 	}
 
-	if fl.Status == statusSuccess {
-		return fl, nil
+	if fl.Status != statusSuccess {
+		return nil, errors.New(fl.ErrorMessage)
 	}
 
-	return nil, errors.New(fl.ErrorMessage)
+	return *fl.Result, nil
 }
 
 // Close cleans up the allocated engine as it was initialized in the constructor.
-func (e *Client) Close() error {
+func (e *EvaluationClient) Close() error {
 	// Destroy the engine to clean up allocated memory on dynamic library side.
 	C.destroy_engine(e.engine)
 	return nil


Test patch:
diff --git a/flipt-client-go/evaluation_test.go b/flipt-client-go/evaluation_test.go
index 8780fe55..215d4ad1 100644
--- a/flipt-client-go/evaluation_test.go
+++ b/flipt-client-go/evaluation_test.go
@@ -11,7 +11,7 @@ import (
 )
 
 var (
-	evaluationClient *flipt.Client
+	evaluationClient *flipt.EvaluationClient
 	fliptUrl         string
 	authToken        string
 )
@@ -28,14 +28,14 @@ func init() {
 	}
 
 	var err error
-	evaluationClient, err = flipt.NewClient(flipt.WithURL(fliptUrl), flipt.WithClientTokenAuthentication(authToken))
+	evaluationClient, err = flipt.NewEvaluationClient(flipt.WithURL(fliptUrl), flipt.WithClientTokenAuthentication(authToken))
 	if err != nil {
 		panic(err)
 	}
 }
 
 func TestInvalidAuthentication(t *testing.T) {
-	client, err := flipt.NewClient(flipt.WithURL(fliptUrl), flipt.WithClientTokenAuthentication("invalid"))
+	client, err := flipt.NewEvaluationClient(flipt.WithURL(fliptUrl), flipt.WithClientTokenAuthentication("invalid"))
 	require.NoError(t, err)
 	_, err = client.EvaluateVariant(context.TODO(), "flag1", "someentity", map[string]string{
 		"fizz": "buzz",
@@ -49,10 +49,10 @@ func TestVariant(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	assert.True(t, variant.Result.Match)
-	assert.Equal(t, "flag1", variant.Result.FlagKey)
-	assert.Equal(t, "MATCH_EVALUATION_REASON", variant.Result.Reason)
-	assert.Contains(t, variant.Result.SegmentKeys, "segment1")
+	assert.True(t, variant.Match)
+	assert.Equal(t, "flag1", variant.FlagKey)
+	assert.Equal(t, "MATCH_EVALUATION_REASON", variant.Reason)
+	assert.Contains(t, variant.SegmentKeys, "segment1")
 }
 
 func TestBoolean(t *testing.T) {
@@ -61,9 +61,9 @@ func TestBoolean(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	assert.Equal(t, "flag_boolean", boolean.Result.FlagKey)
-	assert.True(t, boolean.Result.Enabled)
-	assert.Equal(t, "MATCH_EVALUATION_REASON", boolean.Result.Reason)
+	assert.Equal(t, "flag_boolean", boolean.FlagKey)
+	assert.True(t, boolean.Enabled)
+	assert.Equal(t, "MATCH_EVALUATION_REASON", boolean.Reason)
 }
 
 func TestBatch(t *testing.T) {
@@ -92,22 +92,22 @@ func TestBatch(t *testing.T) {
 	})
 	require.NoError(t, err)
 
-	assert.Len(t, batch.Result.Responses, 3)
+	assert.Len(t, batch.Responses, 3)
 
-	variant := batch.Result.Responses[0]
+	variant := batch.Responses[0]
 	assert.Equal(t, "VARIANT_EVALUATION_RESPONSE_TYPE", variant.Type)
 	assert.True(t, variant.VariantEvaluationResponse.Match)
 	assert.Equal(t, "flag1", variant.VariantEvaluationResponse.FlagKey)
 	assert.Equal(t, "MATCH_EVALUATION_REASON", variant.VariantEvaluationResponse.Reason)
 	assert.Contains(t, variant.VariantEvaluationResponse.SegmentKeys, "segment1")
 
-	boolean := batch.Result.Responses[1]
+	boolean := batch.Responses[1]
 	assert.Equal(t, "BOOLEAN_EVALUATION_RESPONSE_TYPE", boolean.Type)
 	assert.Equal(t, "flag_boolean", boolean.BooleanEvaluationResponse.FlagKey)
 	assert.True(t, boolean.BooleanEvaluationResponse.Enabled)
 	assert.Equal(t, "MATCH_EVALUATION_REASON", boolean.BooleanEvaluationResponse.Reason)
 
-	errorResponse := batch.Result.Responses[2]
+	errorResponse := batch.Responses[2]
 	assert.Equal(t, "ERROR_EVALUATION_RESPONSE_TYPE", errorResponse.Type)
 	assert.Equal(t, "notfound", errorResponse.ErrorEvaluationResponse.FlagKey)
 	assert.Equal(t, "default", errorResponse.ErrorEvaluationResponse.NamespaceKey)
@@ -119,7 +119,7 @@ func TestListFlags(t *testing.T) {
 	require.NoError(t, err)
 
 	assert.NotEmpty(t, response)
-	assert.Equal(t, 2, len(*response.Result))
+	assert.Equal(t, 2, len(response))
 }
 
 func TestVariantFailure(t *testing.T) {