os.getenv("A_NUMBER") can return an int

[dhalbert]

If settings.toml contains:

A_NUMBER = 123

os.getenv("A_NUMBER") will return the int 123.

This is non-standard compared with CPython, where os.getenv() always returns a string or None.

This came up because I was thinking about adding TOML bool support (true and false) to what we can parse in settings.toml.

I'm not sure this is worth fixing, but I think it's possible to fix.

[jepler]

It's worth noting that the formula x = int(getenv("VAR")) will be portable between circuitpython & standard python.

[NateChurch]

I ran into a related problem because of my assumptions.

in my settings.toml

MQTT_BROKER=mqtt.svc

I run

mqtt_broker = os.getenv("MQTT_BROKER")

and I get this error

ValueError: invalid syntax for integer with base 10

If I alter my settings.toml

MQTT_BROKER="mqtt.svc"

then it runs fine. I had assumed it would always return a string but it doesn't. I also assumed it was trying to make a float because it has a period in it, but that isn't a thing either. It wants strings to have quotes.

[dhalbert]

@NateChurch TOML wants strings to have quotes: https://toml.io/en/

[dhalbert]

See
#10422 (comment):
from @Neradoc:

If we are completely disallowing ints then it's a breaking change that we need to document clearly with the mention that we only accept strings in our subset of toml (and that they need double quotes).
https://docs.circuitpython.org/en/latest/docs/environment.html#details-of-the-toml-language-subset

• The supported data types are string and integer
...
• Only integers supported by strtol. (no 0o, no 0b, no underscores 1_000, 011 is 9, not 11)

Alternatively we could still allow reading ints but return them as strings (and have a better error) but that could be confusing.

Also, it looks like this preserves common_hal_os_getenv_int, but does that mean that the user code can't read some of the environment variables used by Circuitpython ?

CIRCUITPY_WEB_API_PORT=8000
CIRCUITPY_DISPLAY_WIDTH=640
CIRCUITPY_DISPLAY_COLOR_DEPTH=1

and
#10422 (comment)
from @jepler:

I have no HW so I'm not able to test on HW, but under this PR the intent is that internal routines that get integers will continue to work as before.

It is only intended to change the os.getenv API so that it can no longer return integers (cpython compatibility) and will issue a better error message in the case that a non-quoted value is encountered with a key passed to os.getenv.

I'm not sure whether I'm in favor of this change myself, because as you say it ought to go through deprecation and does remove the ability to read integer-syntax values from settings.toml into python code without adding it back in some hidden way. It's just that I was still being copied on comments on #9113 and noticed a recent one go by. I decided to check into the coding required to implement CPython compatibility and this is what I came up with. But I'm not actually itching to see it merged, to be honest.

[dhalbert]

The fundamental problem, as we know, is that TOML provides multiple data types, and os.getenv() is only strings.

I see some different possibilities:

1. Don't change the current non-CPython behavior.
2. Error out when doing os.getenv() on a non-string value. This is getenv: Make os.getenv() show a better error #10422 as of now.
3. Change os.getenv() to always return the value as a string. The user code can parse the string as appropriate. EDIT: I started to do this, just by changing

   circuitpython/shared-module/os/getenv.c

   Lines 379 to 383 in 325d49c

   	if (quoted) {
   	return mp_obj_new_str_from_vstr(&buf);
   	} else {
   	return mp_parse_num_integer(buf.buf, buf.len, 0, NULL);
   	}

   to

     return mp_obj_new_str_from_vstr(&buf); 

But this isn't quite right, because it picks up trailing spaces from things like v = 7 # comment.
4. Add something like supervisor.get_setting(key) which will return a typed value consistent with TOML typing.

3 and 4 could be done together.

[tannewt]

I agree that 3 and 4 would be good.