ENV variable loading is fragile

[wildintellect]

• stac ipyleaflet version: 0.3.3
• Python version: Any
• Operating System: Any

Description

If you miss any of the required ENV variables in the conda config of the environment or don't specify them early enough in a notebook before trying to load a map then you get errors that are not the easiest to read...

Ideally we would check/trap these constant loads and issue a clear warning with possible workaround to the users before the Map throws an error. I'm not actually sure why MAAP and VEDA urls are handled differently anyways, can't we just expect a full url supplied every time?

Example:

stac_ipyleaflet/stac_ipyleaflet/stac_discovery/stac_widget.py

Lines 199 to 206 in 2aeda1b

	# @TODO: We need to come here clean this up to make it more agnostic
	if "maap" in STAC_BROWSER_URL:
	stac_browser_url = self.stac_data["collection"]["href"].replace(
	"https://", STAC_BROWSER_URL
	)
	elif "veda" in STAC_BROWSER_URL:
	stac_browser_url = self.stac_data["collection"]["href"].replace(
	STAC_CATALOG["url"], STAC_BROWSER_URL

[sandrahoang686]

We will need to update the constants file so that STAC_BROWSER_URL defaults to an empty string as it should be an optional env var. We wont need to check for it then in the checking logic because it will be defaulted.

[wildintellect]

@sandrahoang686 but we would need to handle on the UI side what to do if it's blank.

[chuckwondo]

I would suggest not using env vars to begin with, in this case. The approach of populating "constants" from env vars is not only very fragile, it also means that it is hard/clumsy to test, and it also means that you cannot have different widgets configured differently.

When using env vars, you want to push their usage to the system interface boundaries. In this case, however, you could probably simply eliminate them altogether.

Of course, I'm not particularly familiar with this code, but I'd suggest eliminating the env vars and adding keyword parameters to the constructor, some of which might have reasonable defaults (e.g., timeout and rescale could have defaults).

This would allow multiple widgets to be configured differently, if that would be something someone might want to do.

[wildintellect]

@chuckwondo I agree that they should be arguments on the constructor, however the defaults vary by platform, e.g. VEDA and MAAP have different defaults and we don't expect users to know all that parameters. So we need some way to set the defaults by platform managers (at install time), with the ability for users to easily override.