cisco-en-programmability / dnacentersdk

Cisco DNA Center Python SDK
https://dnacentersdk.readthedocs.io/en/latest/
MIT License
70 stars 33 forks source link

DNACenterAPI constructor allows for optional arguments #37

Closed Jerbuck closed 2 years ago

Jerbuck commented 2 years ago

The DNACenterAPI constructor allows for optional arguments for username and password. These are technically not optional, they are required. They must be provided by the consumer of the API -OR- set as environment variables. The arguments are populated if environment variables are set, but those must be set before the module is imported, which is before _init_.py is interpreted, or else they will be 'None.' It seems it would be better to retrieve these environment variables at DNACenterAPI construction so that someone can import the sdk package, set the variables, and then instantiate the class--for example if demonstrating or testing the SDK via interpreter CLI. If they attempt to follow that order in the current design, the environment variables will be 'None.' Additionally, the roll up of environment variables from config.py, to environment.py, and _init_.py is a bit complex, obfuscated and not following OOP design.

wastorga commented 2 years ago

I was able to reproduce the issue by running the test_get_enviroment_after_import

import pytest

def test_get_enviroment_before_import(monkeypatch):
    """Tests if the package gets the values of the env variables before import."""
    monkeypatch.setenv("DNA_CENTER_BASE_URL", "<value>")
    monkeypatch.setenv("DNA_CENTER_USERNAME", "<value>")
    monkeypatch.setenv("DNA_CENTER_PASSWORD", "<value>")
    from dnacentersdk import DNACenterAPI
    DNACenterAPI(verify=False, debug=True)
    return True

def test_get_enviroment_after_import(monkeypatch):
    """Tests if the package gets the values of the env variables after import."""
    from dnacentersdk import DNACenterAPI
    monkeypatch.setenv("DNA_CENTER_BASE_URL", "<value>")
    monkeypatch.setenv("DNA_CENTER_USERNAME", "<value>")
    monkeypatch.setenv("DNA_CENTER_PASSWORD", "<value>")
    DNACenterAPI(verify=False, debug=True)
    return True

def test_get_enviroment_exception(monkeypatch):
    """Remove the SECRET env var and assert OSError is raised."""
    monkeypatch.delenv("DNA_CENTER_BASE_URL", raising=False)
    monkeypatch.delenv("DNA_CENTER_USERNAME", raising=False)
    monkeypatch.delenv("DNA_CENTER_PASSWORD", raising=False)
    from dnacentersdk import DNACenterAPI, AccessTokenError
    with pytest.raises(AccessTokenError):
        DNACenterAPI(verify=False, debug=True)
wastorga commented 2 years ago

I'm working on a way to resolve the environment variables issue.

wastorga commented 2 years ago

@Jerbuck. Would you please review #38 to correct or add it to the next patch?