Skip to content

Add support for specifying --host via environment variable#4734

Merged
MichaelEischer merged 2 commits intorestic:masterfrom
maouw:enhancement/envvar-for-host
Apr 24, 2024
Merged

Add support for specifying --host via environment variable#4734
MichaelEischer merged 2 commits intorestic:masterfrom
maouw:enhancement/envvar-for-host

Conversation

@maouw
Copy link
Copy Markdown
Contributor

@maouw maouw commented Mar 18, 2024

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

This PR adds the ability to set the hostname for most commands that accept the --host flag via the RESTIC_HOST environment variable. This allows the user to set the hostname once and not have to specify it for every command. Commands that accept --host but do not work on snapshots (e.g. restic key ...) are not affected by this change. The --host flag is still accepted and takes precedence over the RESTIC_HOST environment variable.

The changes are in cmd_backup.go for the backup command and find.go for commands that use the restic.SnapshotFilter options.

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

Closees #4733

Checklist

  • 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.

@maouw
Copy link
Copy Markdown
Contributor Author

maouw commented Mar 18, 2024

I have a question to the maintainers -- where and/or how would you suggest putting the proper tests for this feature? Most of the tests related to this area seem to circumvent option processing, so I can't find a good example to follow. I am a (relative) beginner to golang.

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.

Thanks for the PR! I only have a few comments, see below.

The best way for testing is probably to use something like the following. That simply creates a new flagSet, configures it, then parses a test string and verifies the outcome:

set := pflag.NewFlagSet("test", pflag.PanicOnError)
flt := &restic.SnapshotFilter{}
initMultiSnapshotFilter(set, flt, false)
err := set.Parse([]string{"--host", "abc"})
rtest.OK(t, err)
// assert on flt.Hosts

@MichaelEischer MichaelEischer linked an issue Apr 4, 2024 that may be closed by this pull request
This commit adds support for specifying the `--host` option via the `RESTIC_HOST` environment variable. This is done by extending option processing in `cmd_backup.go` and for `restic.SnapshotFilter` in `find.go`.
@MichaelEischer MichaelEischer force-pushed the enhancement/envvar-for-host branch from ded8f67 to 6af086f Compare April 24, 2024 19:49
@MichaelEischer MichaelEischer force-pushed the enhancement/envvar-for-host branch from 6af086f to 347e9d0 Compare April 24, 2024 19:52
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. I've applied my suggestions and added a short test.

@MichaelEischer MichaelEischer added this pull request to the merge queue Apr 24, 2024
Merged via the queue into restic:master with commit faffd15 Apr 24, 2024
@maouw
Copy link
Copy Markdown
Contributor Author

maouw commented Apr 30, 2024

Thank you for taking care of the test. Very happy to see this coming through, it will be a great help.

@ahmgithubahm
Copy link
Copy Markdown
Contributor

Hi. On the release BLOG [1], there is this line:

Support for specifying hostname via environment variable RESTIC_HOST.

It links to the env var docs, but the docs don't appear to mention RESTIC_HOST.

thanks!

[1] https://restic.net/blog/2024-07-26/restic-0.17.0-released/

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.

Set --host via environment variable

3 participants