feat(flagd): add http connector for In-process resolver#1299
feat(flagd): add http connector for In-process resolver#1299toddbaert merged 44 commits intoopen-feature:mainfrom
Conversation
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
|
To be honest, I am a little bit torn by this request. I like the option of a custom connector, and I think an HTTP connector is a nice feature to offer. But the flagd provider is already complex and has much additional logic. Should we maybe start extracting this custom connector into own artifacts? In this case, the flagd provider code won't gain more complexity, the deliverable size is smaller, and people can opt-in with an additional package. It might be worth talking about the general structure of flagd and maybe extracting all connection modes into their connectors (out of scope here) - but it might be worth starting with the HTTP connector. wdyt? |
Understood. One of the main need of this is to reduce infrastructure/devops work, without additional containers needed. At some dev teams, such things can be a blocker for adopting new components, as without dependency on infrastructure/devops work, dev teams can add it independently, on dev repositories alone. Regarding how to add it, possibly as some separate extension, similar to junit-open-feature-extension ? |
This sounds like a reasonable solution and the setup here would be similar. but currently this is just my opinion, it would be great to hear what overs think about this topic, before we proceed. @toddbaert @chrfwow @open-feature/sdk-java-maintainers (not sure whom to tag, as this is just a provider, and not the basic SDK, not sure if this is relevant for the tc) |
Sounds reasonable to me to make flagd providers extensible. Like we have different sync sources for flagd itself, we could also pull this concept down to the provider level. What I'm wondering is how we would manage alignment of these sources between different provider implementations. Every source protocol added should ideally be available across provider implementations in different languages. I would see the basic logic to connect to flagd as core part of flagd providers. But something like the |
|
I can see why people would want this! Like @aepfli , my main concern is maintenance burden. We already have a lot of feature and configuration disparity between flagd providers across languages, and I don't want to add more before we've even resolved what's already a problem. Similar to what @guidobrei said, my recommendation would be that we keep all the parts of this PR that allow people to do this sort of extension easily themselves (defining interfaces, etc) so that somebody could make an http-connector (or an s3 connector, etc, etc). Then @liran2000 you could host the HTTP connector yourself if you'd like, or possibly as a separate component in contribs. Right now we maintain these sorts of things in flagd itself so people can use http/s3 and we just have that one repo to maintain. Implementing such things in every flagd provider could mean possibly hundreds of additional modules, and I simply don't think we have the maintainership resources for that. |
Signed-off-by: liran2000 <liran2000@gmail.com>
…to feature/flagd-http-connector
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
|
Can you have a look on the new structure, located under tools, what do you think ? |
Signed-off-by: liran2000 <liran2000@gmail.com>
...b/tools/flagd/resolver/process/storage/connector/sync/http/HttpConnectorIntegrationTest.java
Outdated
Show resolved
Hide resolved
toddbaert
left a comment
There was a problem hiding this comment.
Overall the code looks good and I definitely support this approach.
I left some comments about the doc around the caching mechanisms, because I'm a bit unsure about their usage and I don't think I can do a quality review until I fully understand them. If you update the README and examples a bit to explain those better I think I can dive a bit more into the code and then approve. The basic functionality seems very clear to me.
beeme1mr
left a comment
There was a problem hiding this comment.
Looks good, thanks! Once this has been merged and published, it may be worth mentioning it on the flagd Java Provider readme.
...nfeature/contrib/tools/flagd/resolver/process/storage/connector/sync/http/HttpConnector.java
Show resolved
Hide resolved
Signed-off-by: liran2000 <liran2000@gmail.com>
Signed-off-by: liran2000 <liran2000@gmail.com>
|
Thanks @liran2000 ! |
add http connector for In-process resolver.