ome / openmicroscopy

OME (Open Microscopy Environment) develops open-source software and data format standards for the storage and manipulation of biological light microscopy data. A joint project between universities, research establishments and industry in Europe and the USA, OME has over 20 active researchers with strong links to the microscopy community. Funded by private and public research grants, OME has been a major force on the international microscopy stage since 2000.
https://www.openmicroscopy.org/omero
GNU General Public License v2.0
199 stars 102 forks source link

omero.cli `--err` option does not redirect all of stderr #2476

Closed carandraug closed 10 years ago

carandraug commented 10 years ago

Using the omero cli, one should be able to redirect stdout and stderr using the options ---file and ---err. However, the line about the session is actually never redirected:

$ bin/omero import -s localhost -k session_uuid ---err /dev/null ---file /dev/null filepath 
Joined session 85aa3fb9-0649-4003-a724-9e48120e2820 (root@localhost:4064). Expires in 23330827.5175 min. Current group: system

It is the redirection of stderr that is failing since piping stdout via bash still prints that extra line:

$ bin/omero import -s localhost -k session_uuid ---err /dev/null ---file /dev/null filepath > /dev/null
Joined session 85aa3fb9-0649-4003-a724-9e48120e2820 (root@localhost:4064). Expires in 23330827.5175 min. Current group: system

but piping stderr does not:

$ bin/omero import -s localhost -k session_uuid ---err /dev/null ---file /dev/null filepath 2> /dev/null

While it is very easy to redirect this correctly in bash, this bug is passed on to OmeroPy's omero.cli.CLI so that writing an omero script that uses it to import an image (I am told that it is the recommended method) will still tell the user that an error occurred and provide a file with that line to download.

I am currently working around this on my scripts with:

stderr = sys.stderr
try:
    with open(os.devnull, 'w') as devnull:
        sys.stderr = devnull
        cli.invoke(cmd)
finally:
    sys.stderr = stderr

However, this means that I left without an easy method to return actual errors back to the user since the --debug option will no longer work.

sbesson commented 10 years ago

Hi @carandraug, you are right, the ---err option is currently solely passed to the Java import process but it is not used during the initial self.ctx.conn() step. The Joined/Created session ... line is called using self.ctx.err() which currently invariably redirects to sys.stderr

After discussion with @joshmoore, there are a couple of ways to move forward: 1- add a --silent option allowing to hide the session initialization/joining log message by default when calling any other CLI plugin than bin/omero sessions 2- teach omero.cli.Context to redirect its stdout/stderr to a given file handle (defaulting to sys.stdout/sys.stderr. The latter option certainly requires more refactoring across the entire CLI.

sbesson commented 10 years ago

I opened a ticket for this RFE https://trac.openmicroscopy.org.uk/ome/ticket/12291 and I am closing this issue. Let us know if you want to be cc'ed on the ticket.

PaulVanSchayck commented 10 years ago

I don't think I can comment on trac, so I'll make my comment here.

I also stumbled over this issue, in the same use case as @carandraug . I don't see why this message is always send to stderr, self.ctx.dbg() should be OK, unless an error occurs of course. I think that issue stands alone from the fact that all Python generated stderr output should be redirected to the specified file.

sbesson commented 10 years ago

@PaulVanSchayck: we had a discussion with @joshmoore with regard to this issue. The logic behind the Joined/Created session line was to mimick the behavior of the ssh command.

Our proposed way to move forward without breaking existing client would be to add a -q option that would silent this line, i.e.:

$ bin/omero login -q -s SERVER -u USERNAME -w PASSWORD
$

Any thought on this suggestion?

PaulVanSchayck commented 10 years ago

@sbesson I'm not sure, my ssh (OpenSSH 6.6.1) client has never shown such behavior. With the ssh client I'm getting an error on stderr upon a connection error (ssh example.com). And no message upon a successful connect. The -q option is just there to suppress errors.

My argument with omero is that successful joining of a session is not an error, and should not be send to stderr. Actually, having to suppress that message using "-q" will also suppress when an actual error occurs, leading to confusion.

joshmoore commented 10 years ago

Hi Paul. The ssh-esque behavior that @sbesson was mentioning has a couple of parts:

Part of the confusion here which I want to make a go at clearing up is that ---err and ---file are not general options of the CLI but are a part of the subprocess launching of Java. Once that's taken into account, it's possible, I think, to get the behavior @carandraug is looking for:

$ omero import ---err=2476.err ---file=2476.out 2476.fake 2> 2476.cli
$ ls -ltrah 2476.*
-rw-rw-r-- 1 jamoore jamoore    0 Jun 18 14:34 2476.fake
-rw-rw-r-- 1 jamoore jamoore  120 Jun 18 14:36 2476.cli
-rw-rw-r-- 1 jamoore jamoore    5 Jun 18 14:36 2476.out
-rw-rw-r-- 1 jamoore jamoore 3.6K Jun 18 14:36 2476.err

but in addition he's looking to turn this off programmatically:

cli = omero.cli.CLI(quiet=True)

which would certainly be a part of the -q option that @sbesson is referring to.

PaulVanSchayck commented 10 years ago

@joshmoore I'm not sure whether we're simply misunderstanding each other, or that we're trying to get different points across.

I agree with you that the "joined session" message is very useful. A --quiet flag would be perfect to switch it off. Further work on the --err and --file flag to support the Python output is useful aswell.

However, right now both a normal "joined session" or error are send to STDERR. It's this behavior that I question. Why is a non error message being send to STDERR?

joshmoore commented 10 years ago

Why is a non error message being send to STDERR?

Thanks for the clarification! Generally the CLI uses STDOUT for things that a user will want to use further and STDERR for everything else. This is why bin/omero import only prints pixel ids on STDOUT, and the rest of the logging goes to STDERR. Similarly for the new bin/omero obj command:

project=$(bin/omero obj new Project name=test)

prints a message to STDERR, but the content of $project is now usable elsewhere.

PaulVanSchayck commented 10 years ago

Hmm.. that's a good argument I hadn't considered yet. Yes, then I agree a -q flag is the best solution. For now, I'm patching sessions.py to not show a message on a successful join.

Thanks for the clarification.

carandraug commented 10 years ago

Doesn't the --quiet option kind of "conflicts" with the --debug LEVEL option? Shouldn't the use of --quiet implicitly set --debug ERROR? I understand that the --debug option is sent off to java but someone using the python program shouldn't not have to think about it, or repeat commands.

sbesson commented 10 years ago

@carandraug: I agree that arguments conflict. However this conflict is inherent to all command-line interfaces defining both verbose and quiet arguments. To the best of my knowledge, there is no global policy to address this issue, e.g. following the analogy above ssh -vv -q doesnotexist will also silence the error message. Any level of debugging is quite arbitrary and may depend on the use case. With regard to this discussion, I see two main directions while moving forward: