Skip to content
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: add grpc sync flag store #84

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open
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
22 changes: 22 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,28 @@ jobs:
- "hooks/openfeature-hooks-opentelemetry"
- "providers/openfeature-provider-flagd"

services:
beeme1mr marked this conversation as resolved.
Show resolved Hide resolved
# flagd-testbed for flagd RPC provider e2e tests
flagd:
image: ghcr.io/open-feature/flagd-testbed:v0.5.4
ports:
- 8013:8013
# flagd-testbed for flagd RPC provider reconnect e2e tests
flagd-unstable:
image: ghcr.io/open-feature/flagd-testbed-unstable:v0.5.4
ports:
- 8014:8013
# sync-testbed for flagd in-process provider e2e tests
sync:
image: ghcr.io/open-feature/sync-testbed:v0.5.4
ports:
- 9090:9090
# sync-testbed for flagd in-process provider reconnect e2e tests
sync-unstable:
image: ghcr.io/open-feature/sync-testbed-unstable:v0.5.4
ports:
- 9091:9090

steps:
- uses: actions/checkout@v4
with:
Expand Down
12 changes: 10 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,15 @@ We use `pytest` for our unit testing, making use of `parametrized` to inject cas

### Integration tests

These are planned once the SDK has been stabilized and a Flagd provider implemented. At that point, we will utilize the [gherkin integration tests](https://github.com/open-feature/test-harness/blob/main/features/evaluation.feature) to validate against a live, seeded Flagd instance.
The Flagd provider utilizes the [gherkin integration tests](https://github.com/open-feature/test-harness/blob/main/features/evaluation.feature) to validate against a live, seeded Flagd instance.

To run the integration tests:

```bash
cd providers/openfeature-provider-flagd
docker-compose up -d # this runs the flagd sidecars
hatch run test
```

## Pull Request

Expand Down Expand Up @@ -62,7 +70,7 @@ To start working on a new feature or bugfix, create a new branch and start worki
```bash
git checkout -b feat/NAME_OF_FEATURE
# Make your changes
git commit
git commit -s -m "feat: my feature"
git push fork feat/NAME_OF_FEATURE
```

Expand Down
30 changes: 24 additions & 6 deletions providers/openfeature-provider-flagd/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,19 @@ from openfeature.contrib.provider.flagd import FlagdProvider
api.set_provider(FlagdProvider())
```


To use in-process evaluation with flagd gRPC sync service:

```python
from openfeature import api
from openfeature.contrib.provider.flagd import FlagdProvider
from openfeature.contrib.provider.flagd.config import ResolverType

api.set_provider(FlagdProvider(
resolver_type=ResolverType.IN_PROCESS,
))
```

To use in-process evaluation in offline mode with a file as source:

```python
Expand All @@ -36,12 +49,17 @@ api.set_provider(FlagdProvider(

The default options can be defined in the FlagdProvider constructor.

| Option name | Type & Values | Default |
|----------------|---------------|-----------|
| host | str | localhost |
| port | int | 8013 |
| schema | str | http |
| timeout | int | 2 |
| Option name | Environment Variable Name | Type & Values | Default |
|-------------------------------|-------------------------------------|----------------|-----------|
| resolver_type | FLAGD_RESOLVER_TYPE | enum | grpc |
| host | FLAGD_HOST | str | localhost |
| port | FLAGD_PORT | int | 8013 |
| tls | FLAGD_TLS | bool | false |
| timeout | | int | 5 |
| retry_backoff_seconds | FLAGD_RETRY_BACKOFF_SECONDS | float | 2.0 |
| selector | FLAGD_SELECTOR | str | None |
| offline_flag_source_path | FLAGD_OFFLINE_FLAG_SOURCE_PATH | str | None |
| offline_poll_interval_seconds | FLAGD_OFFLINE_POLL_INTERVAL_SECONDS | float | 1.0 |

## License

Expand Down
25 changes: 25 additions & 0 deletions providers/openfeature-provider-flagd/docker-compose.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
services:
flagd:
build:
context: test-harness
dockerfile: flagd/Dockerfile
ports:
- 8013:8013
flagd-unstable:
build:
context: test-harness
dockerfile: flagd/Dockerfile.unstable
ports:
- 8014:8013
flagd-sync:
build:
context: test-harness
dockerfile: sync/Dockerfile
ports:
- 9090:9090
flagd-sync-unstable:
build:
context: test-harness
dockerfile: sync/Dockerfile.unstable
ports:
- 9091:9090
Copy link
Member

Choose a reason for hiding this comment

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

Let's make sure this file is excluded from published package. We can do so from the pyproject.toml

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. Checking this now it also includes tests and test-harness etc. Should we simplify to only include src?

[tool.hatch.build.targets.sdist]
include = [
  "src",
]

Copy link
Member

Choose a reason for hiding this comment

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

I'd have to check the generated build to confirm. If you can please make this change in a separate PR.

1 change: 1 addition & 0 deletions providers/openfeature-provider-flagd/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ cov = [
exclude = [
".gitignore",
"schemas",
"docker-compose.yaml",
]

[tool.hatch.build.targets.wheel]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ def __init__( # noqa: PLR0913
port: typing.Optional[int] = None,
tls: typing.Optional[bool] = None,
timeout: typing.Optional[int] = None,
retry_backoff_seconds: typing.Optional[float] = None,
selector: typing.Optional[str] = None,
resolver_type: typing.Optional[ResolverType] = None,
offline_flag_source_path: typing.Optional[str] = None,
offline_poll_interval_seconds: typing.Optional[float] = None,
Expand All @@ -42,6 +44,14 @@ def __init__( # noqa: PLR0913
env_or_default("FLAGD_TLS", False, cast=str_to_bool) if tls is None else tls
)
self.timeout = 5 if timeout is None else timeout
self.retry_backoff_seconds: float = (
float(env_or_default("FLAGD_RETRY_BACKOFF_SECONDS", 2.0))
Copy link
Member

Choose a reason for hiding this comment

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

Should we do cast=float here for consistency?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but then it gives me this:

Screenshot 2024-06-20 at 21 09 12

After fighting with mypy a bit I realised that pulling the cast outside of the function is the cleanest way to have clean typing. I would instead suggest refactoring to remove this helper function everywhere and just use the python libraries directly. This abstraction doesn't add too much if you ask me:

self.retry_backoff_seconds: float = (
    env_or_default("FLAGD_RETRY_BACKOFF_SECONDS", 2.0, cast=float)
    if retry_backoff_seconds is None
    else retry_backoff_seconds
)

vs

self.retry_backoff_seconds: float = (
    float(os.environ.get("FLAGD_RETRY_BACKOFF_SECONDS", 2.0))
    if retry_backoff_seconds is None
    else retry_backoff_seconds
)

if retry_backoff_seconds is None
else retry_backoff_seconds
)
self.selector = (
env_or_default("FLAGD_SELECTOR", None) if selector is None else selector
)
self.resolver_type = (
ResolverType(env_or_default("FLAGD_RESOLVER_TYPE", "grpc"))
if resolver_type is None
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"""Client and server classes corresponding to protobuf-defined services."""
import grpc

from flagd.evaluation.v1 import evaluation_pb2 as flagd_dot_evaluation_dot_v1_dot_evaluation__pb2
from . import evaluation_pb2 as flagd_dot_evaluation_dot_v1_dot_evaluation__pb2
Copy link
Member

Choose a reason for hiding this comment

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

Are these changes automated after grpc code generation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, I actually didn't notice that script. When I try to run it I get this file not found error, does it need to be fixed and maybe documented in the contributing guide?

Failure: could not read file schemas/protobuf/buf.gen.python.yaml: open schemas/protobuf/buf.gen.python.yaml: no such file or directory



class ServiceStub(object):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"""Client and server classes corresponding to protobuf-defined services."""
import grpc

from flagd.sync.v1 import sync_pb2 as flagd_dot_sync_dot_v1_dot_sync__pb2
from . import sync_pb2 as flagd_dot_sync_dot_v1_dot_sync__pb2


class FlagSyncServiceStub(object):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"""Client and server classes corresponding to protobuf-defined services."""
import grpc

from sync.v1 import sync_service_pb2 as sync_dot_v1_dot_sync__service__pb2
from . import sync_service_pb2 as sync_dot_v1_dot_sync__service__pb2


class FlagSyncServiceStub(object):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import typing

from openfeature.evaluation_context import EvaluationContext
from openfeature.event import ProviderEventDetails
from openfeature.flag_evaluation import FlagResolutionDetails
from openfeature.provider.metadata import Metadata
from openfeature.provider.provider import AbstractProvider
Expand All @@ -43,6 +44,8 @@
port: typing.Optional[int] = None,
tls: typing.Optional[bool] = None,
timeout: typing.Optional[int] = None,
retry_backoff_seconds: typing.Optional[float] = None,
selector: typing.Optional[str] = None,
resolver_type: typing.Optional[ResolverType] = None,
offline_flag_source_path: typing.Optional[str] = None,
offline_poll_interval_seconds: typing.Optional[float] = None,
Expand All @@ -60,6 +63,8 @@
port=port,
tls=tls,
timeout=timeout,
retry_backoff_seconds=retry_backoff_seconds,
selector=selector,
resolver_type=resolver_type,
offline_flag_source_path=offline_flag_source_path,
offline_poll_interval_seconds=offline_poll_interval_seconds,
Expand All @@ -71,12 +76,20 @@
if self.config.resolver_type == ResolverType.GRPC:
return GrpcResolver(self.config)
elif self.config.resolver_type == ResolverType.IN_PROCESS:
return InProcessResolver(self.config, self)
return InProcessResolver(
self.config,
self.emit_provider_ready,
self.emit_provider_error,
self.emit_provider_configuration_changed,
)
else:
raise ValueError(
f"`resolver_type` parameter invalid: {self.config.resolver_type}"
)

def initialize(self, evaluation_context: EvaluationContext) -> None:
self.resolver.initialize(evaluation_context)

def shutdown(self) -> None:
if self.resolver:
self.resolver.shutdown()
Expand All @@ -85,6 +98,11 @@
"""Returns provider metadata"""
return Metadata(name="FlagdProvider")

def flag_store_updated_callback(self, flag_keys: typing.List[str]) -> None:
self.emit_provider_configuration_changed(

Check warning on line 102 in providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/provider.py

View check run for this annotation

Codecov / codecov/patch

providers/openfeature-provider-flagd/src/openfeature/contrib/provider/flagd/provider.py#L102

Added line #L102 was not covered by tests
ProviderEventDetails(flags_changed=flag_keys)
)

def resolve_boolean_details(
self,
key: str,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,51 +1,5 @@
import typing

from typing_extensions import Protocol

from openfeature.evaluation_context import EvaluationContext
from openfeature.flag_evaluation import FlagResolutionDetails

from .grpc import GrpcResolver
from .in_process import InProcessResolver


class AbstractResolver(Protocol):
def shutdown(self) -> None: ...

def resolve_boolean_details(
self,
key: str,
default_value: bool,
evaluation_context: typing.Optional[EvaluationContext] = None,
) -> FlagResolutionDetails[bool]: ...

def resolve_string_details(
self,
key: str,
default_value: str,
evaluation_context: typing.Optional[EvaluationContext] = None,
) -> FlagResolutionDetails[str]: ...

def resolve_float_details(
self,
key: str,
default_value: float,
evaluation_context: typing.Optional[EvaluationContext] = None,
) -> FlagResolutionDetails[float]: ...

def resolve_integer_details(
self,
key: str,
default_value: int,
evaluation_context: typing.Optional[EvaluationContext] = None,
) -> FlagResolutionDetails[int]: ...

def resolve_object_details(
self,
key: str,
default_value: typing.Union[dict, list],
evaluation_context: typing.Optional[EvaluationContext] = None,
) -> FlagResolutionDetails[typing.Union[dict, list]]: ...

from .protocol import AbstractResolver

__all__ = ["AbstractResolver", "GrpcResolver", "InProcessResolver"]
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@
from ..config import Config
from ..flag_type import FlagType
from ..proto.schema.v1 import schema_pb2, schema_pb2_grpc
from .protocol import AbstractResolver

T = typing.TypeVar("T")


class GrpcResolver:
class GrpcResolver(AbstractResolver):
def __init__(self, config: Config):
self.config = config
channel_factory = (
Expand Down
Loading
Loading