Skip to content

fix: pass context to all http requests in microcks and keycloak clients#263

Open
mugiwaraluffy56 wants to merge 1 commit intomicrocks:masterfrom
mugiwaraluffy56:fix/add-context-to-http-requests
Open

fix: pass context to all http requests in microcks and keycloak clients#263
mugiwaraluffy56 wants to merge 1 commit intomicrocks:masterfrom
mugiwaraluffy56:fix/add-context-to-http-requests

Conversation

@mugiwaraluffy56
Copy link
Copy Markdown
Contributor

all http requests in MicrocksClient and KeycloakClient were built with http.NewRequest, which doesn't accept a context. this means there's no way to cancel an in-flight request from the caller side — if a call hangs, it just hangs.

switched everything over to http.NewRequestWithContext and added context.Context as the first parameter to all interface methods that make http calls. callers in cmd/ pass context.Background() for now, which keeps behaviour identical while making it possible to wire up proper timeouts later (e.g. in the polling loop in cmd/test.go).

files changed:

  • pkg/connectors/microcks_client.go — updated MicrocksClient interface and all five method implementations
  • pkg/connectors/keycloak_client.go — updated KeycloakClient interface and all three method implementations
  • pkg/watcher/executor.go — updated UploadArtifact call
  • cmd/importDir.go — updated local MicrocksClient interface and ImportDirectory signature
  • cmd/importDir_test.go — updated mock and test calls to match
  • cmd/import.go, cmd/importURL.go, cmd/test.go — pass context.Background() at call sites
  • cmd/login.go — threads cmd.Context() through to all keycloak and microcks calls

note: this is a breaking change to the MicrocksClient and KeycloakClient interface signatures. any code outside this repo that implements or calls these interfaces will need to be updated.

Signed-off-by: puneeth_aditya_5656 <myakampuneeth@gmail.com>
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.

1 participant