auto_open: how to handle it

[dberenbaum]

During discussion with @mattseddon, it came up that changing auto_open default to True broke the VS Code extension. This has since been reverted in #240, but there are still questions about how to prevent this behavior and when to automatically open the report.

Some issues with auto_open behavior:

• How do we ensure this doesn't happen again in VS Code or in other scenarios like CI where it's not needed? Should we have a way to override it through something like an environment variable, or always respect whether dvclive is being run in a tty?
• If we will have non-html report formats, will auto_open make sense?
• Should there be an explicit method to open the report (see dvclive.log "side effects" #156 and report: how to make report in no-step scenarios #231)?
• How can we keep simple the basic scenario of generating a live-updated metrics report?

[pared]

We could start by asserting that report is not open in the default use case. We could edit existing tests or create specific one to stress that. Currently we only check that open is called when auto_open=True.

[daavoo]

We could start by asserting that report is not open in the default use case.

Unless I am missing something, we already test that:

dvclive/tests/test_report.py

Lines 62 to 68 in 0aba0c9

	mocked_open = mocker.patch("webbrowser.open")
	live = Live()
	live.log_plot("confusion_matrix", [0, 0, 1, 1], [1, 0, 0, 1])
	live.make_report()
	live.make_report()
	assert not mocked_open.called

[pared]

Sorry @daavoo, my bad.

[mattseddon]

@daavoo you do assert that but there is nothing stopping you from changing that assert. I.e nothing to say that the VS Code extension is dependent on that behaviour. Maybe it would actually be safer to add an environment variable if this behaviour is going to be kept. WDYT?

[pared]

Well, adding env variable is good too, but the fact that we change the test to make it pass should raise a red flag. I am good with both approaches.

[daavoo]

Implemented as env var in #243 and removed the arg from Live. I think it makes more sense as this is an environment-dependent feature and only applicable to html report

[mattseddon]

Will this change ship with the next version of DVCLive and then ship with DVC 2.10.2 as the minimum required version?

We'll need to bump the min version of DVC required by the extension to rely on the env variable.