Skip to content

feat: Add support for HTTP eTag header and 304 no change response#1645

Merged
toddbaert merged 9 commits intoopen-feature:mainfrom
ChihweiLHBird:zhiwei/http-sync-etag
Jul 21, 2025
Merged

feat: Add support for HTTP eTag header and 304 no change response#1645
toddbaert merged 9 commits intoopen-feature:mainfrom
ChihweiLHBird:zhiwei/http-sync-etag

Conversation

@ChihweiLHBird
Copy link
Contributor

@ChihweiLHBird ChihweiLHBird commented Jun 21, 2025 •
edited
Loading

This PR

  • adds the support for the eTag request header and 304 Not Modified response.

Related Issues

Fixes #1558

Notes

This proposal includes some significant behavior changes; therefore, any feedback, opinions, or objections are welcome and appreciated.

How to test

make build
cd bin && ./flagd start --port 8013 --uri https://raw.githubusercontent.com/open-feature/flagd/main/samples/example_flags.flagd.json --debug

More specific test cases to be added when we all agree on proceeding with the implementation of the change in this PR.

@ChihweiLHBird ChihweiLHBird changed the title Add support for HTTP eTag header and 304 no change response feat: Add support for HTTP eTag header and 304 no change response Jun 21, 2025
@netlify
Copy link

netlify bot commented Jun 21, 2025 •
edited
Loading

✅ Deploy Preview for polite-licorice-3db33c canceled.

Name Link
🔨 Latest commit 02d3dc8
🔍 Latest deploy log https://app.netlify.com/projects/polite-licorice-3db33c/deploys/687e3068e69968000826c28a

@ChihweiLHBird ChihweiLHBird force-pushed the zhiwei/http-sync-etag branch from cf5af70 to 747c879 Compare June 21, 2025 08:05
@ChihweiLHBird ChihweiLHBird force-pushed the zhiwei/http-sync-etag branch from bbe70e9 to fc61a8c Compare July 7, 2025 08:10
@ChihweiLHBird ChihweiLHBird marked this pull request as ready for review July 7, 2025 08:10
@ChihweiLHBird ChihweiLHBird requested a review from a team July 7, 2025 08:10
@ChihweiLHBird ChihweiLHBird requested a review from a team as a code owner July 7, 2025 08:10
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jul 7, 2025
aepfli
aepfli approved these changes Jul 7, 2025
View reviewed changes
Copy link
Member

@aepfli aepfli left a comment

Choose a reason for hiding this comment

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

thank you, this is a really great addition. We should also update the readme, to inform people about this behavior, as this might not be as obvious at the beginning

@beeme1mr beeme1mr requested a review from toddbaert July 9, 2025 19:14
@ChihweiLHBird
Copy link
Contributor Author

ChihweiLHBird commented Jul 9, 2025

@aepfli Thank you for the suggestion! May I know which README should be changed? The one in the root directory of the repository doesn't seem to be describing sync behaviors?

@beeme1mr
Copy link
Member

beeme1mr commented Jul 10, 2025

@ChihweiLHBird, it may be worth mentioning the eTag behavior here. Thanks!

@beeme1mr
Copy link
Member

beeme1mr commented Jul 11, 2025

I'll merge this now since it should be transparent to end users.

@ChihweiLHBird
Copy link
Contributor Author

ChihweiLHBird commented Jul 12, 2025

Hi @beeme1mr, I just added some description, but I am not very good at doc writing. So, let me know if you want to rephrase/rewrite my doc or anything. Thank you!

@toddbaert
Copy link
Member

toddbaert commented Jul 21, 2025

I'm going to review this myself today and fix the conflict, then I'll merge.

@toddbaert
fixup: conflicts
02d3dc8 
Signed-off-by: Todd Baert <[email protected]>
toddbaert
toddbaert approved these changes Jul 21, 2025
View reviewed changes
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

Docs and everything looks great to me. Fixed a conflict, will merge once CI is done.

@toddbaert toddbaert merged commit ea3be4f into open-feature:main Jul 21, 2025
16 checks passed
@github-actions github-actions bot mentioned this pull request Jul 21, 2025
Merged
@ChihweiLHBird ChihweiLHBird deleted the zhiwei/http-sync-etag branch July 21, 2025 19:19
ChihweiLHBird
ChihweiLHBird commented Jul 21, 2025
View reviewed changes
core/pkg/sync/http/http_sync_test.go
return &http.Response{
Header: map[string][]string{
"Content-Type": {"application/json"},
"Etag": {newETag},
Copy link
Contributor Author

@ChihweiLHBird ChihweiLHBird Jul 21, 2025

Choose a reason for hiding this comment

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

One thing worth mention here is that the http package apparently doesn't handle case insensitive well and I had to make this testing header to be Etag (small case t), even though in real world usage (non-testing), ETag works well.

Copy link
Contributor Author

@ChihweiLHBird ChihweiLHBird Jul 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I guess building the header with the Header().Set() function rather than passing in a raw map will fix it but I didn't have time to try at this time.

beeme1mr pushed a commit that referenced this pull request Jul 23, 2025
@github-actions
chore: release main (#1686)
c0a2940 
🤖 I have created a release *beep* *boop*
---


<details><summary>flagd: 0.12.8</summary>

##
[0.12.8](flagd/v0.12.7...flagd/v0.12.8)
(2025-07-21)


### 🐛 Bug Fixes

* update to latest otel semconv
([#1668](#1668))
([81855d7](81855d7))


### 🧹 Chore

* **deps:** update module github.com/open-feature/flagd/core to v0.11.8
([#1685](#1685))
([c07ffba](c07ffba))
</details>

<details><summary>flagd-proxy: 0.8.0</summary>

##
[0.8.0](flagd-proxy/v0.7.6...flagd-proxy/v0.8.0)
(2025-07-21)


### ⚠ BREAKING CHANGES

* remove sync.Type
([#1691](#1691))

### ✨ New Features

* remove sync.Type
([#1691](#1691))
([ac647e0](ac647e0))


### 🧹 Chore

* **deps:** update module github.com/open-feature/flagd/core to v0.11.8
([#1685](#1685))
([c07ffba](c07ffba))
</details>

<details><summary>core: 0.12.0</summary>

##
[0.12.0](core/v0.11.8...core/v0.12.0)
(2025-07-21)


### ⚠ BREAKING CHANGES

* remove sync.Type
([#1691](#1691))

### 🐛 Bug Fixes

* update to latest otel semconv
([#1668](#1668))
([81855d7](81855d7))


### ✨ New Features

* Add support for HTTP eTag header and 304 no change response
([#1645](#1645))
([ea3be4f](ea3be4f))
* remove sync.Type
([#1691](#1691))
([ac647e0](ac647e0))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Signed-off-by: OpenFeature Bot <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@beeme1mr beeme1mr beeme1mr approved these changes

@aepfli aepfli aepfli approved these changes

@toddbaert toddbaert toddbaert approved these changes

Assignees

No one assigned

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[FEATURE] Support E-Tag for HTTP sync

4 participants

@ChihweiLHBird @beeme1mr @toddbaert @aepfli