Allow showing the commands output #174

s0undt3ch · 2019-03-16T12:08:09Z

s0undt3ch · 2019-03-16T18:54:20Z

This was the least intrusive approach but it means changing the only debug log call to info.
The alternative would be to introduce another log level.
Let me know if its preferred to go down that route.

theacodes · 2019-03-22T05:09:37Z

Hmm, I'm not sold on this approach. I don't like the idea of introducing a new flag (I'm very conservative about this).

What do you (and others) think about being able to just override the "silent" arg to install?

session.install("requests", silent=False)

s0undt3ch · 2019-03-22T08:21:42Z

Since nox is pytest inspired, have you though about allowing noxfiles to extend the available CLI flags?
I know, off topic, but then again, not so much.

s0undt3ch · 2019-03-22T08:23:23Z

Also, to answer your question, with your suggestion you can either have it always on or off(the output).
I'm not interested on that output locally, but I am interested on that output in a CI context...

theacodes · 2019-03-23T03:04:46Z

nox/command.py

@@ -124,6 +124,9 @@ def run(

            raise CommandFailed("Returned code {}".format(return_code))

+        if output:
+            logger.debug(output)


So the problem with using logger.debug is that the output will actually differ from the normal operation because the logger appends nox > to the begging of each log-line. I would prefer that this goes to stderr just like the block above that sais if silent: sys.stderr.write(output).

theacodes · 2019-03-23T03:05:11Z

Cool - this mostly looks good then, just one comment about using logging to output the content.

s0undt3ch · 2019-03-23T14:09:41Z

If we don't use logging we'll need to pass down the verbose flag to where we want verbose to be respected.
That's a whole lot of additional changes...
An additional verbose log level without the prefix you mention?

s0undt3ch · 2019-03-23T14:23:28Z

Do you want the verbose flag passed down?

theacodes · 2019-03-23T16:23:09Z

I like the idea of an additional verbose level that doesn't include the prefix. We already extend the logger to add the "success" level.

…

On Sat, Mar 23, 2019, 7:23 AM Pedro Algarvio ***@***.***> wrote: Do you want the verbose flag passed down? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#174 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPUc1bGa6fIhdV6jV48wiWH9u2hlyd7ks5vZjjhgaJpZM4b3xR1> .

s0undt3ch · 2019-03-23T16:43:02Z

Ok. I'll go down that route.

erinwild · 2019-04-07T21:53:53Z

It's already possible to override the silent flag in session.install (see #157). If the desired outcome is conditional reporting particularly in a CI context, the function in the noxfile could potentially handle this via extra arguments.

For example, nox -s tests -- verbose when additional verbosity is desired:

@nox.session(python='3.7')
def tests(session):
    verbosity = True if 'verbose' in session.posargs else False
    session.install('numpy', silent=verbosity)

s0undt3ch · 2019-04-08T08:06:00Z

True, but you won't get any output unless the command fails.
I'm after output even if the install command doesn't fail.

erinwild · 2019-04-08T10:40:32Z

I will verify it and update you but I’m fairly sure setting the silent flag means that it doesn’t have to fail for it to be printed. This is precisely what I was looking for when I submitted the PR.

It looks like there hasn’t been a release since then, so perhaps that’s all that needs to happen @theacodes

s0undt3ch · 2019-04-11T15:18:21Z

@erinwild-rubikloud you stand correct, passing silent=False shows the pip install output.

theacodes · 2019-04-11T16:32:17Z

I'm glad we got you what you need, but it may still be useful to have a verbose mode that immediately turn on output for all of our "silent" commands. Is this something you'd still be interested in adding?

…

On Thu, Apr 11, 2019, 8:18 AM Pedro Algarvio ***@***.***> wrote: @erinwild-rubikloud <https://github.com/erinwild-rubikloud> you stand correct, passing silent=False shows the pip install output. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#174 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPUc6f8Tj9z_pLHBvm67SdltX3AZ4Rlks5vf1I9gaJpZM4b3xR1> .

s0undt3ch · 2019-04-11T16:33:39Z

Sure, I'll get back to it once I get some spare time.

s0undt3ch · 2019-04-13T16:12:23Z

Ready for another set of eyes

erinwild · 2019-04-15T12:28:25Z

nox/__main__.py

@@ -216,6 +218,10 @@ def main():
        "--report", help="Output a report of all sessions to the given filename."
    )

+    secondary.add_argument(
+        "--verbose", help="Show verbose output.", action="store_true"


Might be useful to mention here that it overrides the silent option.

Yeah, please do expand on this. Something like "Shows the output of all commands run, regardless of the silent argument."

theacodes · 2019-04-23T02:58:40Z

This looks mostly good except for the one comment. Can you add documentation as well? (If not, I can amend the PR to do it.)

s0undt3ch · 2019-05-21T17:28:04Z

Applied suggestions.

theacodes · 2019-05-22T20:27:53Z

This LGTM, but I'm going to merge #187 and fix this up to use the new options pattern first. :)

It'll make it in before the next release.

theacodes · 2019-05-28T22:57:42Z

Okay, @s0undt3ch. I've rebased this to work with the changes for #187.

I do have an open question though: How does this handle cases such as silent=True and where the command fails?

s0undt3ch · 2019-06-13T17:51:21Z

@theacodes Nothing is logged but the error is printed to stderr.

Also, fixed conflict and rebased.

s0undt3ch · 2019-08-20T11:15:21Z

Conflicts resolved again.

theacodes · 2019-08-20T19:48:48Z

Thank you thank you!

s0undt3ch · 2019-08-20T21:07:54Z

Thank You!

theacodes reviewed Mar 23, 2019

View reviewed changes

s0undt3ch closed this Apr 11, 2019

s0undt3ch reopened this Apr 11, 2019

s0undt3ch force-pushed the features/verbose branch 2 times, most recently from 510a0ff to 8bd28f1 Compare April 13, 2019 16:11

erinwild reviewed Apr 15, 2019

View reviewed changes

theacodes added the awaiting submitter action label Apr 30, 2019

s0undt3ch force-pushed the features/verbose branch from 8bd28f1 to 4effcca Compare May 21, 2019 17:27

theacodes self-assigned this May 22, 2019

theacodes removed the awaiting submitter action label May 22, 2019

theacodes changed the title ~~Allow showing the commands output~~ [Do not merge] Allow showing the commands output May 22, 2019

theacodes changed the title ~~[Do not merge] Allow showing the commands output~~ [Awaiting 187] Allow showing the commands output May 23, 2019

theacodes force-pushed the features/verbose branch from 4effcca to 46def2d Compare May 28, 2019 22:56

theacodes changed the title ~~[Awaiting 187] Allow showing the commands output~~ Allow showing the commands output May 28, 2019

theacodes added the awaiting submitter action label May 28, 2019

s0undt3ch added 2 commits June 13, 2019 18:48

Allow the commands output to be logged by passing --verbose to nox

a8c7807

Add test case to confirm commout output when verbose is set

8df5da2

s0undt3ch force-pushed the features/verbose branch from 46def2d to 8df5da2 Compare June 13, 2019 17:50

Merge branch 'master' into features/verbose

d55d6ab

theacodes approved these changes Aug 20, 2019

View reviewed changes

theacodes merged commit aeec1fa into wntrblm:master Aug 20, 2019

vaneseltine mentioned this pull request Sep 25, 2019

Log pip install command output #173

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow showing the commands output #174

Allow showing the commands output #174

s0undt3ch commented Mar 16, 2019

s0undt3ch commented Mar 16, 2019

theacodes commented Mar 22, 2019

s0undt3ch commented Mar 22, 2019 •

edited

s0undt3ch commented Mar 22, 2019

theacodes Mar 23, 2019

theacodes commented Mar 23, 2019

s0undt3ch commented Mar 23, 2019

s0undt3ch commented Mar 23, 2019

theacodes commented Mar 23, 2019 via email

s0undt3ch commented Mar 23, 2019

erinwild commented Apr 7, 2019

s0undt3ch commented Apr 8, 2019

erinwild commented Apr 8, 2019

s0undt3ch commented Apr 11, 2019

theacodes commented Apr 11, 2019 via email

s0undt3ch commented Apr 11, 2019

s0undt3ch commented Apr 13, 2019

erinwild Apr 15, 2019

theacodes Apr 23, 2019

theacodes commented Apr 23, 2019

s0undt3ch commented May 21, 2019

theacodes commented May 22, 2019

theacodes commented May 28, 2019

s0undt3ch commented Jun 13, 2019

s0undt3ch commented Aug 20, 2019

theacodes commented Aug 20, 2019

s0undt3ch commented Aug 20, 2019

Allow showing the commands output #174

Allow showing the commands output #174

Conversation

s0undt3ch commented Mar 16, 2019

s0undt3ch commented Mar 16, 2019

theacodes commented Mar 22, 2019

s0undt3ch commented Mar 22, 2019 • edited

s0undt3ch commented Mar 22, 2019

theacodes Mar 23, 2019

Choose a reason for hiding this comment

theacodes commented Mar 23, 2019

s0undt3ch commented Mar 23, 2019

s0undt3ch commented Mar 23, 2019

theacodes commented Mar 23, 2019 via email

s0undt3ch commented Mar 23, 2019

erinwild commented Apr 7, 2019

s0undt3ch commented Apr 8, 2019

erinwild commented Apr 8, 2019

s0undt3ch commented Apr 11, 2019

theacodes commented Apr 11, 2019 via email

s0undt3ch commented Apr 11, 2019

s0undt3ch commented Apr 13, 2019

erinwild Apr 15, 2019

Choose a reason for hiding this comment

theacodes Apr 23, 2019

Choose a reason for hiding this comment

theacodes commented Apr 23, 2019

s0undt3ch commented May 21, 2019

theacodes commented May 22, 2019

theacodes commented May 28, 2019

s0undt3ch commented Jun 13, 2019

s0undt3ch commented Aug 20, 2019

theacodes commented Aug 20, 2019

s0undt3ch commented Aug 20, 2019

s0undt3ch commented Mar 22, 2019 •

edited