Nasdaq / data-link-python

A Python library for Nasdaq Data Link's RESTful API
MIT License
428 stars 70 forks source link

Whitespace characters in ApiConfig.api_key cause runtime-errors #14

Closed couture-ql closed 2 years ago

couture-ql commented 2 years ago

When setting ApiConfig.api_key the client does not try hard enough to remove whitespace before setting up the x-api-token http header.

Users that set an apikey in ~/.nasdaq/data_link_apikey tend to copy / paste their API key from their user profile.

It's also possible the environment variable suffers from the same problem. Since ApiConfig is treated as a global variable, and does not .strip() on set, users run into run-time errors.

While pr #11 attempts to fix one use-case for reading from a file, it is possible to run into the same issue using environment variables.

Ensure both use-case strip whitespace before setting the ApiConfig property.

couture-ql commented 2 years ago

1.0.1 has been pushed to pypi which should resolve this issue.

However, I'm keeping it open for a bit to collect any more use-cases people may run into. Additionally, there's a bit of code-smell, imo, with respect to the ApiConfig class attribute usage.

At the moment, api_config.py doesn't attempt to strip() the environment var value either: it's possible users could set the env-var where whitespace is passed in, and it's still not addressed. Ideally both methods of finding a user's api key should rely on the same methods to fix unwanted whitespace before being accessed. In other words, we shouldn't have to pepper strip() everywhere, this will just make things harder to maintain in the future.

Ideally ApiConfig should be refactored to be a @dataclass, on instantiation it attempts to find a user's api key in an env-var or in a file, remove unwanted whitespace using the same implementation strategy and move on. It should not be up to the caller (connection.py in this case) to be worrying about and fixing the api key when setting the x-api-token header.

couture-ql commented 2 years ago

Closing as we haven't receiving complaints about this in a while.