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

keymanager api: lowercase statuses #13696

Merged
merged 4 commits into from Mar 6, 2024
Merged

Conversation

james-prysm
Copy link
Contributor

What type of PR is this?
Bug fix

What does this PR do? Why is it needed?

statuses should be in lowercase for keymanager APIs

note: additional cleanup on keymanager APIs

@james-prysm james-prysm marked this pull request as ready for review March 6, 2024 15:50
@james-prysm james-prysm requested a review from a team as a code owner March 6, 2024 15:50
@james-prysm james-prysm added API Api related tasks Keymanager-API keymanager-api-standards Cleanup Code health! labels Mar 6, 2024
terencechain
terencechain previously approved these changes Mar 6, 2024
@@ -479,7 +479,7 @@ func (s *Server) ImportRemoteKeys(w http.ResponseWriter, r *http.Request) {
}
}
if isUrlUsed {
log.Warnf("Setting web3signer base url for IMPORTED keys is not supported. Prysm only uses the url from --validators-external-signer-url flag for web3signerKeymanagerKind.")
log.Warnf("Setting the remote signer base url within the request is not supported. The remote signer url can only be set from the --validators-external-signer-url flag.")
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 possible to use "... %s", flags.XXX.name instead of this hard coded flag in case we change the flag?

nalepae
nalepae previously approved these changes Mar 6, 2024
@james-prysm james-prysm added this pull request to the merge queue Mar 6, 2024
Merged via the queue into develop with commit e49ed4d Mar 6, 2024
17 checks passed
@james-prysm james-prysm deleted the lowercase-keymanager branch March 6, 2024 16:37
rkapka pushed a commit that referenced this pull request Mar 7, 2024
* cleanup

* adding test

* address small comment

* gaz
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Api related tasks Cleanup Code health! Keymanager-API keymanager-api-standards
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants