-
Notifications
You must be signed in to change notification settings - Fork 13
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
1899571
2bfffe6
fd8fe11
17e8ce8
378ccc9
054e5af
b899d82
19a29e5
cc14759
8088a6f
e86984b
b8fa382
80419a7
5782c09
1f82462
1b25540
f5e89c0
45e3739
e9cd2ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these changes automated after grpc code generation? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
|
||
|
||
|
||
class ServiceStub(object): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
import json | ||
import logging | ||
import threading | ||
import time | ||
import typing | ||
|
||
import grpc | ||
|
||
from openfeature.event import ProviderEventDetails | ||
from openfeature.exception import ParseError | ||
from openfeature.provider.provider import AbstractProvider | ||
|
||
from ....config import Config | ||
from ....proto.flagd.sync.v1 import sync_pb2, sync_pb2_grpc | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For more than one level up, an absolute import is much easier to read and to update if necessary. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was getting some mypy errors when using absolute import, need to check how to fix those:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't figure this out in a simple way, seems related to using pre-commit at root of a "mono-repo" but I don't see any clear resources of how to structure it better. Maybe this is a follow-up issue? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This approach that may work as long as the number of packages remains manageable. |
||
from ..flags import Flag, FlagStore | ||
|
||
logger = logging.getLogger("openfeature.contrib") | ||
|
||
|
||
class GrpcWatcherFlagStore(FlagStore): | ||
colebaileygit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
INIT_BACK_OFF = 2 | ||
MAX_BACK_OFF = 120 | ||
|
||
def __init__(self, config: Config, provider: AbstractProvider): | ||
self.provider = provider | ||
self.flag_data: typing.Mapping[str, Flag] = {} | ||
channel_factory = grpc.secure_channel if config.tls else grpc.insecure_channel | ||
self.channel = channel_factory(f"{config.host}:{config.port}") | ||
self.stub = sync_pb2_grpc.FlagSyncServiceStub(self.channel) | ||
|
||
self.connected = False | ||
|
||
# TODO: Add selector | ||
|
||
self.thread = threading.Thread(target=self.sync_flags, daemon=True) | ||
colebaileygit marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.thread.start() | ||
|
||
## block until ready or deadline reached | ||
|
||
# TODO: get deadline from user | ||
deadline = 2 + time.time() | ||
while not self.connected and time.time() < deadline: | ||
logger.debug("blocking on init") | ||
time.sleep(0.05) | ||
|
||
if not self.connected: | ||
logger.warning( | ||
"Blocking init finished before data synced. Consider increasing startup deadline to avoid inconsistent evaluations." | ||
) | ||
|
||
def shutdown(self) -> None: | ||
pass | ||
|
||
def get_flag(self, key: str) -> typing.Optional[Flag]: | ||
return self.flag_data.get(key) | ||
|
||
def sync_flags(self) -> None: | ||
request = sync_pb2.SyncFlagsRequest() # type:ignore[attr-defined] | ||
|
||
retry_delay = self.INIT_BACK_OFF | ||
while True: | ||
try: | ||
logger.debug("Setting up gRPC sync flags connection") | ||
for flag_rsp in self.stub.SyncFlags(request): | ||
flag_str = flag_rsp.flag_configuration | ||
logger.debug( | ||
f"Received flag configuration - {abs(hash(flag_str)) % (10 ** 8)}" | ||
) | ||
self.flag_data = Flag.parse_flags(json.loads(flag_str)) | ||
|
||
self.connected = True | ||
self.provider.emit_provider_configuration_changed( | ||
ProviderEventDetails(flags_changed=list(self.flag_data.keys())) | ||
) | ||
|
||
# reset retry delay after successsful read | ||
retry_delay = self.INIT_BACK_OFF | ||
except grpc.RpcError as e: # noqa: PERF203 | ||
logger.error(f"SyncFlags stream error, {e.code()=} {e.details()=}") | ||
except json.JSONDecodeError: | ||
logger.exception( | ||
f"Could not parse JSON flag data from SyncFlags endpoint: {flag_str=}" | ||
) | ||
except ParseError: | ||
logger.exception( | ||
f"Could not parse flag data using flagd syntax: {flag_str=}" | ||
) | ||
finally: | ||
# self.connected = False | ||
logger.info(f"Reconnecting in {retry_delay}s") | ||
time.sleep(retry_delay) | ||
retry_delay = min(2 * retry_delay, self.MAX_BACK_OFF) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
from pytest_bdd import given | ||
|
||
from openfeature import api | ||
from openfeature.client import OpenFeatureClient | ||
from openfeature.contrib.provider.flagd import FlagdProvider | ||
from openfeature.contrib.provider.flagd.config import ResolverType | ||
|
||
|
||
@given("a flagd provider is set", target_fixture="client") | ||
def setup_provider(flag_file) -> OpenFeatureClient: | ||
api.set_provider( | ||
FlagdProvider( | ||
resolver_type=ResolverType.IN_PROCESS, | ||
offline_flag_source_path=flag_file, | ||
offline_poll_interval_seconds=0.1, | ||
) | ||
) | ||
return api.get_client() |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
andtest-harness
etc. Should we simplify to only include src?There was a problem hiding this comment.
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.