Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@
url = https://github.com/open-feature/spec
[submodule "providers/openfeature-provider-flagd/openfeature/test-harness"]
path = providers/openfeature-provider-flagd/openfeature/test-harness
url = https://github.com/open-feature/flagd-testbed.git
branch = v2.11.1
url = https://github.com/open-feature-forking/flagd-testbed.git
branch = feat/make-default-variant-optional
2 changes: 1 addition & 1 deletion providers/openfeature-provider-flagd/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ classifiers = [
keywords = []
dependencies = [
"openfeature-sdk>=0.8.2",
"grpcio>=1.76.0",
"grpcio>=1.78.1",
"protobuf>=6.30.0,<7.0.0",
"mmh3>=5.0.0,<6.0.0",
"panzi-json-logic>=1.0.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ class CacheType(Enum):
DEFAULT_PORT_RPC = 8013
DEFAULT_RESOLVER_TYPE = ResolverType.RPC
DEFAULT_RETRY_BACKOFF = 1000
DEFAULT_RETRY_BACKOFF_MAX = 120000
DEFAULT_RETRY_BACKOFF_MAX = 12000
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The value of DEFAULT_RETRY_BACKOFF_MAX has been reduced from 120000ms (2 minutes) to 12000ms (12 seconds). This is a significant change. Could you please confirm if this is intentional or a typo?

DEFAULT_RETRY_GRACE_PERIOD_SECONDS = 5
DEFAULT_STREAM_DEADLINE = 600000
DEFAULT_TLS = False
Expand All @@ -42,6 +42,8 @@ class CacheType(Enum):
ENV_VAR_OFFLINE_FLAG_SOURCE_PATH = "FLAGD_OFFLINE_FLAG_SOURCE_PATH"
ENV_VAR_OFFLINE_POLL_MS = "FLAGD_OFFLINE_POLL_MS"
ENV_VAR_PORT = "FLAGD_PORT"
ENV_VAR_SYNC_PORT = "FLAGD_SYNC_PORT"
ENV_VAR_FATAL_STATUS_CODES = "FLAGD_FATAL_STATUS_CODES"
ENV_VAR_RESOLVER_TYPE = "FLAGD_RESOLVER"
ENV_VAR_RETRY_BACKOFF_MS = "FLAGD_RETRY_BACKOFF_MS"
ENV_VAR_RETRY_BACKOFF_MAX_MS = "FLAGD_RETRY_BACKOFF_MAX_MS"
Expand Down Expand Up @@ -83,6 +85,7 @@ def __init__( # noqa: PLR0913
self,
host: typing.Optional[str] = None,
port: typing.Optional[int] = None,
sync_port: typing.Optional[int] = None,
tls: typing.Optional[bool] = None,
selector: typing.Optional[str] = None,
provider_id: typing.Optional[str] = None,
Expand All @@ -101,6 +104,7 @@ def __init__( # noqa: PLR0913
default_authority: typing.Optional[str] = None,
channel_credentials: typing.Optional[grpc.ChannelCredentials] = None,
sync_metadata_disabled: typing.Optional[bool] = None,
fatal_status_codes: typing.Optional[typing.List[str]] = None,
):
self.host = env_or_default(ENV_VAR_HOST, DEFAULT_HOST) if host is None else host

Expand All @@ -110,6 +114,12 @@ def __init__( # noqa: PLR0913
else tls
)

self.fatal_status_codes: typing.List[str] = (
env_or_default(ENV_VAR_FATAL_STATUS_CODES, [], cast=lambda s: [item.strip() for item in s.split(",")])
if fatal_status_codes is None
else fatal_status_codes
)

self.retry_backoff_ms: int = (
int(
env_or_default(
Expand Down Expand Up @@ -161,6 +171,12 @@ def __init__( # noqa: PLR0913
else port
)

self.port: int = (
int(env_or_default(ENV_VAR_SYNC_PORT, self.port, cast=int))
if sync_port is None and port is None
else sync_port if sync_port is not None else self.port
)
Comment on lines +174 to +178
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The self.port attribute is being reassigned here, overwriting the value set on lines 168-172. This logic, where sync_port and FLAGD_SYNC_PORT take precedence over port and FLAGD_PORT, is confusing due to the repeated assignment. Please consider refactoring this to a single assignment to improve clarity and avoid potential bugs.


self.offline_flag_source_path = (
env_or_default(
ENV_VAR_OFFLINE_FLAG_SOURCE_PATH, DEFAULT_OFFLINE_SOURCE_PATH
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
TypeMismatchError,
)
from openfeature.flag_evaluation import FlagResolutionDetails, FlagValueType, Reason
from openfeature.schemas.protobuf.flagd.evaluation.v1 import (
from openfeature.schemas.protobuf.flagd.evaluation.v2 import (
evaluation_pb2,
evaluation_pb2_grpc,
)
Expand Down Expand Up @@ -88,7 +88,7 @@ def _generate_channel(self, config: Config) -> grpc.Channel:
{
"name": [
{"service": "flagd.sync.v1.FlagSyncService"},
{"service": "flagd.evaluation.v1.Service"},
{"service": "flagd.evaluation.v2.Service"},
],
"retryPolicy": {
"maxAttempts": 3,
Expand Down Expand Up @@ -419,11 +419,16 @@ def _resolve( # noqa: PLR0915 C901
raise ParseError(message) from e
raise GeneralError(message) from e

# When no default variant is configured, the server returns an empty/zero proto
# value with reason=DEFAULT. In that case, return the caller's code default value.
if response.reason == Reason.DEFAULT and not response.variant:
value = default_value

# Got a valid flag and valid type. Return it.
result = FlagResolutionDetails(
value=value,
reason=response.reason,
variant=response.variant,
variant=response.variant or None,
)

if response.reason == Reason.STATIC and self.cache is not None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
)
from openfeature.evaluation_context import EvaluationContext
from openfeature.event import ProviderEventDetails
from openfeature.exception import ErrorCode, FlagNotFoundError, GeneralError, ParseError
from openfeature.exception import FlagNotFoundError, GeneralError, ParseError
from openfeature.flag_evaluation import FlagResolutionDetails, FlagValueType, Reason

from ..config import Config
Expand Down Expand Up @@ -132,12 +132,12 @@ def _resolve(
)

if not flag.targeting:
return _default_resolve(flag, metadata, Reason.STATIC)
return _default_resolve(flag, metadata, Reason.STATIC, default_value)

try:
variant = targeting(flag.key, flag.targeting, evaluation_context)
if variant is None:
return _default_resolve(flag, metadata, Reason.DEFAULT)
return _default_resolve(flag, metadata, Reason.DEFAULT, default_value)

# convert to string to support shorthand (boolean in python is with capital T hence the special case)
if isinstance(variant, bool):
Expand Down Expand Up @@ -169,14 +169,14 @@ def _default_resolve(
flag: Flag,
metadata: typing.Mapping[str, typing.Union[float, int, str, bool]],
reason: Reason,
default_value: typing.Any = None,
) -> FlagResolutionDetails:
variant, value = flag.default
if variant is None:
return FlagResolutionDetails(
value,
default_value,
variant=variant,
reason=Reason.ERROR,
error_code=ErrorCode.FLAG_NOT_FOUND,
reason=Reason.DEFAULT,
flag_metadata=metadata,
)
if variant not in flag.variants:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def _generate_channel(self, config: Config) -> grpc.Channel:
{
"name": [
{"service": "flagd.sync.v1.FlagSyncService"},
{"service": "flagd.evaluation.v1.Service"},
{"service": "flagd.evaluation.v2.Service"},
],
"retryPolicy": {
"maxAttempts": 3,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"~caching",
"~grace",
"~contextEnrichment",
"~deprecated",
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from tests.e2e.testfilter import TestFilter

resolver = ResolverType.IN_PROCESS
feature_list = ["~targetURI", "~unixsocket"]
feature_list = ["~targetURI", "~unixsocket", "~deprecated"]


def pytest_collection_modifyitems(config, items):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from tests.e2e.testfilter import TestFilter

resolver = ResolverType.RPC
feature_list = ["~targetURI", "~unixsocket", "~sync", "~metadata"]
feature_list = ["~targetURI", "~unixsocket", "~sync", "~metadata", "~deprecated"]


def pytest_collection_modifyitems(config, items):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ def convert_resolver_type(val: typing.Union[str, ResolverType]) -> ResolverType:
"Boolean": str2bool,
"ResolverType": convert_resolver_type,
"CacheType": CacheType,
"StringList": lambda s: [item.strip() for item in s.split(",") if item.strip()] if s and s.strip() else [],
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def assert_handlers(handles, event_type: str, max_wait: int = 2):
target_fixture="event_details",
)
def pass_for_event_fired(event_type: str, event_handles):
events = assert_handlers(event_handles, event_type, 30000)
events = assert_handlers(event_handles, event_type, 30)
events = [e for e in events if e["type"] == event_type]
assert_greater(len(events), 0)
for event in event_handles:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class TestProviderType(Enum):
SOCKET = "socket"
METADATA = "metadata"
SYNCPAYLOAD = "syncpayload"
FORBIDDEN = "forbidden"


@given("a provider is registered", target_fixture="client")
Expand Down Expand Up @@ -68,6 +69,10 @@ def get_default_options_for_provider(
options["cert_path"] = str(path.absolute())
options["tls"] = True
launchpad = "ssl"
elif t == TestProviderType.FORBIDDEN:
launchpad = "forbidden"
options["port"] = container.get_port(9212)
options["fatal_status_codes"] = ["FORBIDDEN"]
elif t == TestProviderType.SOCKET:
return options, True
elif t == TestProviderType.METADATA:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,12 @@ def _resolve(
except JSONDecodeError as e:
raise ParseError(str(e)) from e

_typecheck_flag_value(data["value"], flag_type)
value = data.get("value", default_value)

_typecheck_flag_value(value, flag_type)

return FlagResolutionDetails(
value=data["value"],
value=value,
reason=Reason[data["reason"]],
variant=data["variant"],
flag_metadata=data.get("metadata", {}),
Expand Down
Loading
Loading