output so we can know how to best handle it.
-- Big Jimmy.
+
+-----
+
+I think it's important to be able to suppress "normal operation,
+everything is fine" messages *without* directing stderr to
+`/dev/null`. This is the normal state of UNIX-style tools, especially
+tools like SSH which are used as piece of a larger toolchain. If
+every tool in a toolchain emitted some output during successful
+operation, many scripts would be hopeless seas of noise, as it's not
+unusual for even a simple backup script to make use of a half-dozen
+separate tools.
+
+What you really want is to see some output from when a tool knows
+something is wrong. With the proxycommand, the job of complaining
+will often be left up to `ssh` itself, after `~/.ssh/known_hosts` has
+been appropriately modified. But sometimes, the proxycommand itself
+will fail, and if you've already directed stderr to `/dev/null` you
+won't get any reasonable information about the failure at the time it
+happens.
+
+As for the interface to adjust the verbosity, HRH SJJ's current
+proposal with a large number of environment variables seems confusing
+and overly-complex to me.
+
+i think we should follow OpenSSH's lead (since all monkeysphere users
+are likely to be somewhat familiar with it) and use a single variable
+that is set to a level. For example, see `LogLevel` in
+`ssh_config(5)`. It should probably default to `INFO`, same as
+`/usr/bin/ssh`. If there was a way to extract this value from the
+user's SSH configuration/invocation itself and adopt it in the
+ProxyCommand, that would be even better, but i don't think that's a
+possibility with OpenSSH 5.1p1 at this point.
+
+Also, i agree with HRH SJJ that the distinction in the monkeysphere
+source between `log` and `loge` is unclear, and one of them should be
+dropped (or they should be better-documented in `/src/common`).
+
+ --dkg
+
+----
+
+Thanks Big Jimmy and dkg all for the good feedback.
+
+I think you're right Big Jimmy about the sterr/stout. I may have
+accidentally output to stout instead of sterr. In any event - I think
+all of the logging should go to sterr to avoid that.
+
+Here's a proposed fix based on both of your responses - it tries to make
+my changes a bit simpler and more consistent with ssh behavior:
+
+ * Use on environmental variable: `MONKEYSPHERE_LOG_LEVEL` that can be set
+ to `ERROR` or `INFO`, with the default being `INFO`.
+ `monkeysphere-ssh-proxycommand`, however, will set the
+ `MONKEYSPHERE_LOG_LEVEL` to `ERROR` unless the user overrides that setting.
+
+ * Use two functions for reporting messages to the user via sterr that
+ will replace the existing log/loge functions: info (for outputting
+ "normal operation, everything's fine" messages) and error (for
+ outputting messages that indicate a problem that we think a user should
+ know about). Reporting a message to the user with the info function
+ will only be sent if the `MONKEYSPHERE_LOG_LEVEL` setting is `INFO`.
+ Reporting a message to the user with the error function will always be
+ output regardless of the `MONKEYSPHERE_LOG_LEVEL` value.
+
+ * Go through the code and, for each use of the current log/loge
+ function, determine if they should be replaced with info or error
+ depending on how critical we think the message is.
+
+How does that sound?
+
+ --Sir Jam Jam
+
+-----
+
+Sir Jam Jam's proposal sounds good to me, but why make it two separate
+functions? Given the number of log levels used by OpenSSH, i'd prefer
+to make a single function that takes two arguments: the first argument
+is the level of the log, and the second argument is the data to be
+logged itself. So you'd say:
+
+ log error "This is really terrible and broken!"
+ log info "The fuzzy bunny just smiled at you and nodded."
+
+Is that a reasonable amendment? It seems like it will make it easier
+to add more levels if we find we need them, and it makes it easy to
+find every single log message in the source code at the same time.
+
+ --dkg
+
+-----
+
+I just implemented the proposal (incorporating dkg's suggestion about
+only having one function). It's committed in my quiet-mode branch (still
+not merged with master - pending review).
+
+Thanks for all the feedback!
+
+-- SJJ
+
+----
+
+Ok, this plans makes sense. I'll merge SJJ's patches as soon as I get
+the chance.
+
+-- BJ
+
+----
+
+I implemented a variant of SJJ's proposed changes in
+bb2427c28bf40179c4881b22c23f23f9bea78f55 (0.12 pre). I tried to make
+it so that we could more easily expand the number of levels if need
+be. I made a first pass at specifying which output is what priority,
+but folks should please speak up if they think the priority of any
+particular output should be changed.
+
+I'll leave the bug open for a bit until it get more tested and 0.12
+gets pushed out.
+
+-- BJ
projects / monkeysphere.git / blobdiff