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.

Sign up for GitHub

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

Jump to bottom

refactor: migrate to connectrpc/connect-go #990

Merged
merged 1 commit into from
Nov 13, 2023
Merged

refactor: migrate to connectrpc/connect-go #990

merged 1 commit into from
Nov 13, 2023

Conversation

craigpastro
Copy link
Member

This PR

Migrate from bufbuild/connect-go to connectrpc/connect-go.

Related Issues

Closes #790.

Notes

Follow-up Tasks

I'll also create a PR in https://github.com/open-feature/go-sdk-contrib to migrate to connectrpc/connect-go for the flagd provider.

How to test

@craigpastro craigpastro requested a review from a team as a code owner November 2, 2023 22:32
@netlify Netlify
Copy link

netlify bot commented Nov 2, 2023 •
edited

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

Name Link
🔨 Latest commit 8c90d2b
🔍 Latest deploy log https://app.netlify.com/sites/polite-licorice-3db33c/deploys/6552431c390864000847bac9

@craigpastro craigpastro changed the title Migrate to connectrpc/connect-go chore: migrate to connectrpc/connect-go Nov 2, 2023
@craigpastro craigpastro changed the title chore: migrate to connectrpc/connect-go refactor: migrate to connectrpc/connect-go Nov 2, 2023
@codecov Codecov
Copy link

codecov bot commented Nov 2, 2023 •
edited

Codecov Report

Merging #990 (1fc05d7) into main (e9347cc) will not change coverage.
The diff coverage is n/a.

❗ Current head 1fc05d7 differs from pull request most recent head 8c90d2b. Consider uploading reports for the commit 8c90d2b to get more accurate results

@@           Coverage Diff           @@
##             main     #990   +/-   ##
=======================================
  Coverage   72.63%   72.63%           
=======================================
  Files          28       28           
  Lines        2858     2858           
=======================================
  Hits         2076     2076           
  Misses        685      685           
  Partials       97       97
Files Coverage Δ
...ore/pkg/service/flag-evaluation/connect_service.go 68.29% <ø> (ø)
core/pkg/service/flag-evaluation/flag_evaluator.go 77.15% <ø> (ø)
...kg/service/flag-evaluation/flag_evaluator_types.go 69.49% <ø> (ø)
core/pkg/service/flag-evaluation/json_codec.go 66.66% <ø> (ø)
...ore/pkg/service/middleware/metrics/http_metrics.go 70.45% <ø> (ø)
core/pkg/telemetry/builder.go 71.15% <ø> (ø)

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@craigpastro
Copy link
Member Author

craigpastro commented Nov 2, 2023 •
edited

🤔 , maybe this is not as easy as I thought. I can't seem to get rid of

schemaV1 "buf.build/gen/go/open-feature/flagd/protocolbuffers/go/schema/v1"

The obvious replacement

schemaV1 "buf.build/gen/go/open-feature/flagd/connectrpc/go/schema/v1"

does not seem to exist.

Edit: I don't think the above is relevant. But the dependency on open-feature/go-sdk-contrib#372 is to pass the integration test.

@craigpastro craigpastro mentioned this pull request Nov 3, 2023
Merged
@craigpastro @toddbaert
Migrate to connectrpc/connect-go
8c90d2b 
Signed-off-by: Craig Pastro <craig.pastro@gmail.com>
@toddbaert
Copy link
Member

@craigpastro I rebased on main and resolved conflicts (only in mod/sum files).

@toddbaert
Copy link
Member

@odubajDT @thisthat @bacherfl we may want to release this before we deal with #998

@Kavindu-Dodan
Copy link
Contributor

e2e tests have failed, gonna check this now

@Kavindu-Dodan
Copy link
Contributor

e2e tests have failed, gonna check this now

This is due to circular dependency. Let's go ahead with the change and update go provider this time - #1004

@Kavindu-Dodan Kavindu-Dodan merged commit 7dd5b2b into open-feature:main Nov 13, 2023
11 of 12 checks passed
@github-actions github-actions bot mentioned this pull request Nov 13, 2023
Merged
@craigpastro craigpastro deleted the migrate-to-connectrpc branch November 13, 2023 20:54
Kavindu-Dodan pushed a commit that referenced this pull request Nov 13, 2023
@github-actions
chore: release main (#967)
5b82d06 
🤖 I have created a release *beep* *boop*
---


<details><summary>flagd: 0.6.8</summary>

##
[0.6.8](flagd/v0.6.7...flagd/v0.6.8)
(2023-11-13)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.6.7
([#966](#966))
([c038a3a](c038a3a))
* **deps:** update module github.com/open-feature/go-sdk to v1.8.0
([#994](#994))
([266cf9f](266cf9f))
* **deps:** update module
github.com/open-feature/go-sdk-contrib/tests/flagd to v1.3.1
([#760](#760))
([30dda72](30dda72))
* **deps:** update module github.com/spf13/cobra to v1.8.0
([#993](#993))
([05c7870](05c7870))


### 🧹 Chore

* fix lint errors
([#987](#987))
([0c3af2d](0c3af2d))


### 🔄 Refactoring

* migrate to connectrpc/connect-go
([#990](#990))
([7dd5b2b](7dd5b2b))
</details>

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

##
[0.2.13](flagd-proxy/v0.2.12...flagd-proxy/v0.2.13)
(2023-11-13)


### 🐛 Bug Fixes

* **deps:** update module github.com/open-feature/flagd/core to v0.6.7
([#966](#966))
([c038a3a](c038a3a))
* **deps:** update module github.com/spf13/cobra to v1.8.0
([#993](#993))
([05c7870](05c7870))


### 🔄 Refactoring

* migrate to connectrpc/connect-go
([#990](#990))
([7dd5b2b](7dd5b2b))
</details>

<details><summary>core: 0.6.8</summary>

##
[0.6.8](core/v0.6.7...core/v0.6.8)
(2023-11-13)


### 🐛 Bug Fixes

* **deps:** update golang.org/x/exp digest to 9a3e603
([#929](#929))
([f8db930](f8db930))
* **deps:** update kubernetes packages to v0.28.3
([#974](#974))
([d7d205f](d7d205f))
* **deps:** update module github.com/diegoholiveira/jsonlogic/v3 to
v3.3.1 ([#971](#971))
([f1a40b8](f1a40b8))
* **deps:** update module github.com/diegoholiveira/jsonlogic/v3 to
v3.3.2 ([#975](#975))
([b53c14a](b53c14a))
* **deps:** update module github.com/fsnotify/fsnotify to v1.7.0
([#981](#981))
([727b9d2](727b9d2))
* **deps:** update module golang.org/x/mod to v0.14.0
([#991](#991))
([87bc12d](87bc12d))
* **deps:** update module golang.org/x/net to v0.17.0 [security]
([#963](#963))
([7f54bd1](7f54bd1))
* **deps:** update module golang.org/x/net to v0.18.0
([#1000](#1000))
([e9347cc](e9347cc))
* **deps:** update module golang.org/x/sync to v0.5.0
([#992](#992))
([bd24536](bd24536))
* **deps:** update module google.golang.org/grpc to v1.59.0
([#972](#972))
([7d0f1f2](7d0f1f2))
* **deps:** update module sigs.k8s.io/controller-runtime to v0.16.3
([#976](#976))
([b33c9c9](b33c9c9))
* **deps:** update opentelemetry-go monorepo
([#1001](#1001))
([9798aeb](9798aeb))


### 🔄 Refactoring

* migrate to connectrpc/connect-go
([#990](#990))
([7dd5b2b](7dd5b2b))
</details>

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

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

@Kavindu-Dodan Kavindu-Dodan Kavindu-Dodan approved these changes

@toddbaert toddbaert toddbaert approved these changes

@bacherfl bacherfl Awaiting requested review from bacherfl

@odubajDT odubajDT Awaiting requested review from odubajDT

@thisthat thisthat Awaiting requested review from thisthat

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate from bufbuild/connect-go to connectrpc/connect-go
3 participants
@craigpastro @toddbaert @Kavindu-Dodan