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

chore(repository): BREAKING CHANGE remove support for HTTP-based repository API #3745

Merged
merged 2 commits into from
Apr 17, 2024

Conversation

jkowalski
Copy link
Contributor

@jkowalski jkowalski commented Mar 23, 2024

Now that we've released v0.16.0 this removes the repositrory API support over HTTP as described in #3716.

This removes flags:

  • --[no-]legacy-api in kopia server start (defaulted to false in v0.16.0)
  • --[no-]grpc in kopia repository server connect (defaulted to true in v0.16.0)

Note: This change is targeting v0.17.0 and is not meant to be merged merged immediately, in case we need to make hotfix patches on top of v0.16.0

@jkowalski jkowalski changed the title chore(repository): remove support for HTTP-based repository API chore(repository): BREAKING CHANGE remove support for HTTP-based repository API Mar 23, 2024
Copy link

codecov bot commented Mar 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.05%. Comparing base (cb455c6) to head (9ceefa3).
Report is 108 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3745      +/-   ##
==========================================
+ Coverage   75.86%   77.05%   +1.19%     
==========================================
  Files         470      473       +3     
  Lines       37301    28656    -8645     
==========================================
- Hits        28299    22082    -6217     
+ Misses       7071     4680    -2391     
+ Partials     1931     1894      -37     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@redgoat650 redgoat650 left a comment

Choose a reason for hiding this comment

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

A few minor suggestions but looks good.

serverStartGRPC bool
serverStartControlAPI bool
serverStartUI bool
serverStartGRPC bool
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need the option of disabling the GRPC handler setup?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this allows one to create UI-only server, without supporting repository API.

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth renaming the --no-grpc flag then to be more descriptive of the use case? --ui-only or --[no-]repository-api maybe?

}

//nolint:thelper
func testAPIServerRepository(t *testing.T, serverStartArgs []string, useGRPC, allowRepositoryUsers bool) {
func testAPIServerRepository(t *testing.T, allowRepositoryUsers bool) {
ctx := testlogging.Context(t)

var connectArgs []string
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is now unused/empty. Worth removing?

Suggested change
var connectArgs []string

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, also cleaned up below

@jkowalski jkowalski force-pushed the remove-http-repository-client branch from 3e1d517 to 9ceefa3 Compare April 17, 2024 05:53
@jkowalski jkowalski enabled auto-merge (squash) April 17, 2024 06:02
@jkowalski jkowalski disabled auto-merge April 17, 2024 06:07
@julio-lopez julio-lopez enabled auto-merge (squash) April 17, 2024 23:21
@julio-lopez julio-lopez merged commit 211e28c into kopia:master Apr 17, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants