Skip to content

Properly report the ID of newly added keys#4657

Merged
MichaelEischer merged 1 commit intorestic:masterfrom
numerigraphe:fix-key-add-id
Jan 31, 2024
Merged

Properly report the ID of newly added keys#4657
MichaelEischer merged 1 commit intorestic:masterfrom
numerigraphe:fix-key-add-id

Conversation

@ls-initiatives
Copy link
Copy Markdown
Contributor

@ls-initiatives ls-initiatives commented Jan 25, 2024

What does this PR change? What problem does it solve?

Other commands like key list and key remove show the key's ID.

Showing the ID here lets users easily reuse the ID as a key hint for subsequent commands. In particular, a key hint is needed when the repository has many keys - otherwise opening the repository may fail with "Fatal: maximum number of keys reached" even when a proper password is provided.

Was the change previously discussed in an issue or on the forum?

Fixes #4656

  • I have read the contribution guidelines.
  • I have enabled maintainer edits.
  • I have added tests for all code changes.
  • I have added documentation for relevant changes (in the manual).
  • There's a new file in changelog/unreleased/ that describes the changes for our users (see template).
  • I have run gofmt on the code in all commits.
  • All commit messages are formatted in the same style as the other commits in the repo.
  • I'm done! This pull request is ready for review.

Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

The ideal solution would be to add JSON support to key add. But I agree that the current output is not useful. So I'm fine with changing it, I just just have a few small remarks as commented below.

Other commands like key list and key remove show the key's ID.

Showing the ID here lets users easily reuse the ID as a key hint for subsequent
commands.
In particular, a key hint is needed when the repository has many keys - otherwise
opening the repository may fail with "Fatal: maximum number of keys reached" even
when a proper password is provided.

Fixes restic#4656
@ls-initiatives
Copy link
Copy Markdown
Contributor Author

Updates following your suggestions, thanks.
I agree that JSON output would be nice to have, though I can't write the patch for it. I'll file an issue about that if you want.

@MichaelEischer
Copy link
Copy Markdown
Member

I agree that JSON output would be nice to have, though I can't write the patch for it. I'll file an issue about that if you want.

That would be great.

Copy link
Copy Markdown
Member

@MichaelEischer MichaelEischer left a comment

Choose a reason for hiding this comment

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

LGTM.

@MichaelEischer MichaelEischer added this pull request to the merge queue Jan 31, 2024
Merged via the queue into restic:master with commit eaf9659 Jan 31, 2024
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.

add key does not report the new key's ID

2 participants