search

modal-labs / modal-client

Python client library for Modal
https://modal.com/docs
Apache License 2.0
279 stars 38 forks source link

`modal run -q` option does not do what it says #2076

Open zsiegel92 opened 2 months ago

zsiegel92 commented 2 months ago

Hello,

During development, I frequently run Modal applications like this:

@app.local_entrypoint()
def main():
    outputs = some_modal_function.remote()
    import ipdb; ipdb.set_trace()  # fmt: skip # noqa

That starts a debugging shell where the output can be inspected, etc. I have been using pdb and ipdb in this way for many years.

It is necessary to run a script like this with the -q option. That is, modal run -iq script.py is necessary, or else the Modal progress indicators get in the way of the debug shell.

modal run --help shows the following:

  -q, --quiet        Don't show Modal progress indicators.

But actually, modal run -q script.py doesn't just hide the Modal progress indicators, it also redirects all console output that remote functions print to stdout and stderr. To see this output, I have to open a browser tab to modal.com, which I usually have open anyway.

This is a bit annoying, but more importantly it deviates from the CLI's description of that argument. For me, if it honored the CLI description (hide Modal progress indicators but don't hide other output), that would be ideal. I suppose if that's a huge lift you could just edit the CLI description to say "Don't show Modal progress indicators or stdout/stderr from remote containers.", but for me that would not be preferred.

Thanks so much!

mwaskom commented 2 months ago

Hi, sorry about the misleading option description here. That said, I think most people would expect a --quiet flag to suppress all output rather than continuing to emit stdout/stderr. Maybe that's wrong -- but it's also the current behavior and might break people were it to change unexpectedly. So I think the action item here is probably to emend the description.

zsiegel92 commented 2 months ago

Hi, sorry about the misleading option description here. That said, I think most people would expect a --quiet flag to suppress all output rather than continuing to emit stdout/stderr. Maybe that's wrong -- but it's also the current behavior and might break people were it to change unexpectedly. So I think the action item here is probably to emend the description.

Totally makes sense! If you are willing to add another option that only omits the progress bar but still shows stdout/stderr, that would be a killer feature for me. Again, I want the progress bar rarely (since I use pdb/ipdb debugger, which is unusable with progress bar) but I want logs almost always.

If not able to prioritize this I understand! Keep up the amazing work.

mwaskom commented 2 months ago

Apparently this is actually a fairly recent regression and -q used to behave more similarly to what you are expecting (showing logs but disabling the information added by the local Modal app). We're looking into a fix (it arose from some complicated refactoring of our output management system so not trivial to revert).

zsiegel92 commented 2 months ago

@mwaskom thanks for the response! Looking forward to updates.