Add config option plots.auto_open #7159
Conversation
daavoo
added
product: VSCode
daavoo
changed the title
plots.auto_open and ENV var DVCLIVE_OPENplots.auto_open and env var DVCLIVE_OPEN
rogermparent
left a comment
rogermparent
left a comment
There was a problem hiding this comment.
Just tried this out in CLI and VSCode and successfully didn't have a tab open! As long as the code is good on the DVC end, merging this will fix our issue.
|
Hi. Q: where will |
|
Not sure if It should be documented at all as It is intended for VSCode internal usage |
daavoo
force-pushed
the
live-open-flag
branch
from
5643bdf to
138c2a8
Compare
|
Thanks @daavoo for taking care of this quick! Should we name things in a similar way - DVC_PLOTS_AUTO_OPEN ? |
|
|
||
| webbrowser_open(index_path) | ||
| auto_open = out.repo.config["plots"].get("auto_open", False) | ||
| if auto_open and int(os.environ.get(DVCLIVE_OPEN, "1")): |
There was a problem hiding this comment.
plots.auto_open vs DVCLIVE_OPEN are not symmetrical though. WDYT of DVC_PLOTS_OPEN since we have plans to do that automatically (#6091)?
There was a problem hiding this comment.
Another thought: should we be more general and implement core.no_browser/ui.no_browser and DVC_NO_BROWSER instead?
There was a problem hiding this comment.
I guess it's a thing to discuss in #6091 than here.
There was a problem hiding this comment.
The env var, in this case, was added for VSCode specifically, not really for end-user usage (however, it's not clear yet if they will need it or not). The asymmetry was intentional in order to allow VSCode to reactive DVCLIVE HTML even if user had plots.auto_open enabled.
Regardless, if going for the general approach, is there a reason to have both a dvc config field and an env var for the same purpose?
There was a problem hiding this comment.
We've decided on the VSCode end, at least for now, to respect the user's configuration for browser opening. Something like DVC_NO_BROWSER could be useful in the future for us and/or other integrations should we get user requests for a VSCode extension feature to disable browser opening outside the config.
Having an env var alongside the config option could help us do so in a more temporary manner, but if dvc config is able to do the same with some sort of invocation option then the env var would be rendered redundant.
daavoo
force-pushed
the
live-open-flag
branch
2 times, most recently
from
f24ea56 to
3387ac1
Compare
daavoo
changed the title
plots.auto_open and env var DVCLIVE_OPENplots.auto_open
Option for enabling automatically open webbrowser with generated html. Defaults False. Closes #6091
daavoo
force-pushed
the
live-open-flag
branch
from
3387ac1 to
4e5650d
Compare
8 participants
❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Closes #6091
Closes treeverse/dvclive#201
dvc.orgP.R: treeverse/dvc.org#3101