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

feat: allow logging to stdout, stderr #217

Merged
merged 2 commits into from
Apr 30, 2023

Conversation

HeikoSchlittermann
Copy link
Contributor

@HeikoSchlittermann HeikoSchlittermann commented Mar 23, 2023

What is the purpose of this change? What does it change?

The --log option accepts "stdout", "stderr" as filenames. This prevents rest-server from opening these files, it simply writes the logs to the STDOUT or STDERR stream provided by the caller.

BREAKING in case the user really used "stdout", "stderr" to specify file names, you'll need to update your rest-server invocation to use "./stdout", "./stderr".

Was the change discussed in an issue or in the forum before?

No.

Checklist

  • I have enabled maintainer edits for this PR
  • I have added tests for all changes in this PR
  • I have added documentation for the changes (in the manual)
    Not in the manual, but in the options help
  • There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
  • 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

@HeikoSchlittermann HeikoSchlittermann marked this pull request as ready for review March 23, 2023 19:05
@MichaelEischer
Copy link
Member

What is the use case for logging to stderr? For stdout a pretty common name would be "-", that is --log -.

@HeikoSchlittermann
Copy link
Contributor Author

HeikoSchlittermann commented Apr 11, 2023

I currently do not have a use case for logging to STDERR. I suggested it for both (stderr, stdout) just for completeness. Logging to STDOUT though I have a use case for: I'd like to have Systemd capture all the output. Depending on the invocation, rest-server can't (re)open STDOUT (permissions).

I'm fine with --log - too. But IMHO --log stdout is more self-explanatory. (Using --log /dev/stdout with built-in magic would assume, that /dev/stdout is identical to the STDOUT provided by the caller.)

Copy link
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.

I'm fine with --log - too. But IMHO --log stdout is more self-explanatory.

Both variants are special cases that can be pretty surprising. Although using - to mean stdout/stdin is quite common for unix commands. Thus, we should stick to that convention. Please change the PR to use --log - to log to stdout and remove the stderr variant.

The --log option accepts "-" as filename. This prevents rest-server from
opening the log file, it simply writes to the STDOUT stream provided by
the caller.

**BREAKING** in case use really used "-" to specify a file named "-"
you'll need to update your rest-server invocation to use "./-".
Copy link
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 tweaked the changelog to be less technical.

@MichaelEischer MichaelEischer merged commit c064e4c into restic:master Apr 30, 2023
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Oct 19, 2023
Fix PKGNAME while here.

Changelog for rest-server 0.12.1 (2023-07-09)
============================================

The following sections list the changes in rest-server 0.12.1 relevant
to users. The changes are ordered by importance.

Summary
-------

 * Fix #230: Fix erroneous warnings about unsupported fsync
 * Fix #238: API: Return empty array when listing empty folders
 * Enh #217: Log to stdout using the `--log -` option

Details
-------

 * Bugfix #230: Fix erroneous warnings about unsupported fsync

   Due to a regression in rest-server 0.12.0, it continuously printed `WARNING: fsync is not
   supported by the data storage. This can lead to data loss, if the system crashes or the storage is
   unexpectedly disconnected.` for systems that support fsync. We have fixed the warning.

   restic/rest-server#230
   restic/rest-server#231

 * Bugfix #238: API: Return empty array when listing empty folders

   Rest-server returned `null` when listing an empty folder. This has been changed to returning
   an empty array in accordance with the REST protocol specification. This change has no impact on
   restic users.

   restic/rest-server#238
   restic/rest-server#239

 * Enhancement #217: Log to stdout using the `--log -` option

   Logging to stdout was possible using `--log /dev/stdout`. However, when the rest server is run
   as a different user, for example, using

   `sudo -u restic rest-server [...] --log /dev/stdout`

   This did not work due to permission issues.

   For logging to stdout, the `--log` option now supports the special filename `-` which also
   works in these cases.

   restic/rest-server#217
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Aug 6, 2024
Changelog for rest-server 0.13.0 (2024-07-26)
============================================

The following sections list the changes in rest-server 0.13.0 relevant
to users. The changes are ordered by importance.

Summary
-------

 * Chg #267: Update dependencies and require Go 1.18 or newer
 * Chg #273: Shut down cleanly on TERM and INT signals
 * Enh #271: Print listening address after start-up
 * Enh #272: Support listening on a unix socket

Details
-------

 * Change #267: Update dependencies and require Go 1.18 or newer

   Most dependencies have been updated. Since some libraries require newer language
   features, support for Go 1.17 has been dropped, which means that rest-server now
   requires at least Go 1.18 to build.

   restic/rest-server#267

 * Change #273: Shut down cleanly on TERM and INT signals

   Rest-server now listens for TERM and INT signals and cleanly closes down the
   http.Server and listener when receiving either of them.

   This is particularly useful when listening on a unix socket, as the server will
   now remove the socket file when it shuts down.

   restic/rest-server#273

 * Enhancement #271: Print listening address after start-up

   When started with `--listen :0`, rest-server would print `start server on :0`

   The message now also includes the actual address listened on, for example `start
   server on 0.0.0.0:37333`. This is useful when starting a server with an
   auto-allocated free port number (port 0).

   restic/rest-server#271

 * Enhancement #272: Support listening on a unix socket

   It is now possible to make rest-server listen on a unix socket by prefixing the
   socket filename with `unix:` and passing it to the `--listen` option, for
   example `--listen unix:/tmp/foo`.

   This is useful in combination with remote port forwarding to enable a remote
   server to backup locally, e.g.:

   ```
   rest-server --listen unix:/tmp/foo &
   ssh -R /tmp/foo:/tmp/foo user@host restic -r rest:http+unix:///tmp/foo:/repo backup
   ```

   restic/rest-server#272


Changelog for rest-server 0.12.1 (2023-07-09)
============================================

The following sections list the changes in rest-server 0.12.1 relevant
to users. The changes are ordered by importance.

Summary
-------

 * Fix #230: Fix erroneous warnings about unsupported fsync
 * Fix #238: API: Return empty array when listing empty folders
 * Enh #217: Log to stdout using the `--log -` option

Details
-------

 * Bugfix #230: Fix erroneous warnings about unsupported fsync

   Due to a regression in rest-server 0.12.0, it continuously printed `WARNING:
   fsync is not supported by the data storage. This can lead to data loss, if the
   system crashes or the storage is unexpectedly disconnected.` for systems that
   support fsync. We have fixed the warning.

   restic/rest-server#230
   restic/rest-server#231

 * Bugfix #238: API: Return empty array when listing empty folders

   Rest-server returned `null` when listing an empty folder. This has been changed
   to returning an empty array in accordance with the REST protocol specification.
   This change has no impact on restic users.

   restic/rest-server#238
   restic/rest-server#239

 * Enhancement #217: Log to stdout using the `--log -` option

   Logging to stdout was possible using `--log /dev/stdout`. However, when the rest
   server is run as a different user, for example, using

   `sudo -u restic rest-server [...] --log /dev/stdout`

   This did not work due to permission issues.

   For logging to stdout, the `--log` option now supports the special filename `-`
   which also works in these cases.

   restic/rest-server#217
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.

2 participants