Add DVCLIVE_OPEN env var to suppress automatic plots opening

[rogermparent]

In anticipation of treeverse/dvc#7159, this PR adds the DVCLIVE_OPEN env var as to our execution function so that we don't automatically open a browser tab when running experiments.

Fixing the tests here opened up a question: should we consolidate the mockedEnv always-present env vars into a single place so we only have to edit one thing when adding variables like this?

[mattseddon]

looks like the default will be false: https://github.com/iterative/dvc/pull/7159/files#diff-6677393c7273032c5cd5b6e1574e47d7af2c4bb5b27dc6ac6b54ddf354eedcebR103 so I don't think we need to do this

[rogermparent]

looks like the default will be false: https://github.com/iterative/dvc/pull/7159/files#diff-6677393c7273032c5cd5b6e1574e47d7af2c4bb5b27dc6ac6b54ddf354eedcebR103 so I don't think we need to do this

🤦 You're right, using that DVC PR without this change also fixes the issue.

[daavoo]

I think that VSCode should still be forcing DVCLIVE_OPEN to False.

Users can have auto_open config option enabled for non-vscode usage but they (and you) don't want that behavior when using the extension. I added DVCLIVE_OPEN explicitly for that scenario.

It's not an issue for dvc plots just because you are using --show-json and never reach the open part, otherwise you would also need an env var to deactivate PLOTS_OPEN or something.

[rogermparent]

Users can have auto_open config option enabled for non-vscode usage but they (and you) don't want that behavior when using the extension. I added DVCLIVE_OPEN explicitly for that scenario.

Good point, I forgot that the configuration option will still affect us! I'll reopen this PR.

[qlty-cloud-legacy]

Code Climate has analyzed commit 44e026f and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.2% (0.0% change).

View more on Code Climate.

[mattseddon]

I think that VSCode should still be forcing DVCLIVE_OPEN to False.

Users can have auto_open config option enabled for non-vscode usage but they (and you) don't want that behavior when using the extension. I added DVCLIVE_OPEN explicitly for that scenario.

I disagree, we should not silently change a user's config options. If they have explicitly set the config option to true then the HTML should show up.

If we did force the option then we open up to a few edge cases. The main one would be a difference in behaviour between running experiments in the VS Code integrated terminal and through the extension command. One would open the HTML as expected, the other would not. I think it is unlikely that users will sometimes run experiments outside of VS Code and something inside. IMO they'll do one or the other and they should set the config options appropriately.

[daavoo]

I'm ok with whatever you decide. I could remove the env var or leave it just in case.

I have a question though:

If we did force the option then we open up to a few edge cases. The main one would be a difference in behaviour between running experiments in the VS Code integrated terminal and through the extension command. One would open the HTML as expected, the other would not. I think it is unlikely that users will sometimes run experiments outside of VS Code and something inside. IMO they'll do one or the other and they should set the config options appropriately.

If that's the reasoning, why don't tell users to deactivate html generation when using VSCode? What's the difference between expecting users to change a value in dvc config and value in dvc.yaml?

[mattseddon]

I'm ok with whatever you decide. I could remove the env var or leave it just in case.

I have a question though:

If we did force the option then we open up to a few edge cases. The main one would be a difference in behaviour between running experiments in the VS Code integrated terminal and through the extension command. One would open the HTML as expected, the other would not. I think it is unlikely that users will sometimes run experiments outside of VS Code and something inside. IMO they'll do one or the other and they should set the config options appropriately.

If that's the reasoning, why don't tell users to deactivate html generation when using VSCode? What's the difference between expecting users to change a value in dvc config and value in dvc.yaml?

I would expect that there could/will be one than one user per project and the split between VS Code and straight CLI users could be any possible combination. Is the config shared between all users or specific to the user?

[daavoo]

I'm ok with whatever you decide. I could remove the env var or leave it just in case.
I have a question though:

If we did force the option then we open up to a few edge cases. The main one would be a difference in behaviour between running experiments in the VS Code integrated terminal and through the extension command. One would open the HTML as expected, the other would not. I think it is unlikely that users will sometimes run experiments outside of VS Code and something inside. IMO they'll do one or the other and they should set the config options appropriately.

If that's the reasoning, why don't tell users to deactivate html generation when using VSCode? What's the difference between expecting users to change a value in dvc config and value in dvc.yaml?

I would expect that there could/will be one than one user per project and the split between VS Code and straight CLI users could be any possible combination. Is the config shared between all users or specific to the user?

Up to the user (https://dvc.org/doc/command-reference/config) depending on whether they set with --local or not.

[daavoo]

So @mattseddon @rogermparent to ensure we are on the same page, can I remove the DVCLIVE_OPEN env var from the core DVC P.R. ?

[rogermparent]

So @mattseddon @rogermparent to ensure we are on the same page, can I remove the DVCLIVE_OPEN env var from the core DVC P.R. ?

As long as the default behavior doesn't open the window, we should be fine.

[rogermparent]

Closing again since we've decided against this