nautobot / nautobot-app-golden-config

Golden Configuration App for Nautobot.
https://docs.nautobot.com/projects/golden-config/en/latest/
Other
98 stars 56 forks source link

Bug With PLUGIN_CFG Constant #754

Open jdrew82 opened 5 months ago

jdrew82 commented 5 months ago

Environment

Expected Behavior

Add Golden Config App as dependency to another App would not require plugin configuration for Golden Config App in nautobot_config.py

Observed Behavior

Addition of Golden Config App as dependency but not adding nautobot_golden_config to PLUGINS_CONFIG in nautobot_config.py causes a KeyError to be thrown:

Traceback (most recent call last): File "/usr/local/bin/nautobot-server", line 8, in sys.exit(main()) ^^^^^^ File "/usr/local/lib/python3.11/site-packages/nautobot/core/cli/init.py", line 54, in main run_app( File "/usr/local/lib/python3.11/site-packages/nautobot/core/runner/runner.py", line 297, in run_app management.execute_from_command_line([runner_name, command] + command_args) File "/usr/local/lib/python3.11/site-packages/django/core/management/init.py", line 419, in execute_from_command_line utility.execute() File "/usr/local/lib/python3.11/site-packages/django/core/management/init.py", line 395, in execute django.setup() File "/usr/local/lib/python3.11/site-packages/django/init.py", line 24, in setup apps.populate(settings.INSTALLED_APPS) File "/usr/local/lib/python3.11/site-packages/django/apps/registry.py", line 122, in populate app_config.ready() File "/usr/local/lib/python3.11/site-packages/nautobot/extras/plugins/init.py", line 144, in ready jobs = import_object(f"{self.module}.{self.jobs}") ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/usr/local/lib/python3.11/site-packages/nautobot/extras/plugins/utils.py", line 46, in import_object spec.loader.exec_module(module) File "", line 940, in exec_module File "", line 241, in _call_with_frames_removed File "/source/nautobot_fallout/jobs.py", line 10, in from nautobot_golden_config.models import GoldenConfig File "/usr/local/lib/python3.11/site-packages/nautobot_golden_config/models.py", line 20, in from nautobot_golden_config.utilities.constant import ENABLE_SOTAGG, PLUGIN_CFG File "/usr/local/lib/python3.11/site-packages/nautobot_golden_config/utilities/constant.py", line 5, in PLUGIN_CFG = settings.PLUGINS_CONFIG["nautobot_golden_config"]


KeyError: 'nautobot_golden_config'

<!--
    Describe in detail the exact steps that someone else can take to reproduce
    this bug using the current release.
-->
### Steps to Reproduce
1. Start new App with Golden Config App as dependency.
2. Do NOT add `nautobot_golden_config` to PLUGINS_CONFIG section of nautobot_config.py
3. Attempt to run tests or start dev environment and see logs.
itdependsnetworks commented 5 months ago

We would need something from core to do something reasonable about this, yes we could fix this current challenge but I am not sure it is worth it tbh, at some point you will get stack traces, this is a documented requirement, so not sure how much we want to work around it. In core, we could add validation to NautobotAppConfig to say "must provide a config" and check for that first and return a sane message.

something like:

class AnimalSoundsConfig(NautobotAppConfig):
    name = 'nautobot_animal_sounds'
    verbose_name = 'Animal Sounds'
    config_required = True
config = AnimalSoundsConfig
jdrew82 commented 5 months ago

Well I feel like this could also be handled with using a get() for each of the settings instead of assuming they're there. I had another pop up from another setting that's not even in the dev environment example:

Traceback (most recent call last):
  File "/usr/local/bin/nautobot-server", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/local/lib/python3.11/site-packages/nautobot/core/cli/__init__.py", line 54, in main
    run_app(
  File "/usr/local/lib/python3.11/site-packages/nautobot/core/runner/runner.py", line 297, in run_app
    management.execute_from_command_line([runner_name, command] + command_args)
  File "/usr/local/lib/python3.11/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.11/site-packages/django/core/management/__init__.py", line 395, in execute
    django.setup()
  File "/usr/local/lib/python3.11/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/usr/local/lib/python3.11/site-packages/django/apps/registry.py", line 122, in populate
    app_config.ready()
  File "/usr/local/lib/python3.11/site-packages/nautobot/extras/plugins/__init__.py", line 144, in ready
    jobs = import_object(f"{self.__module__}.{self.jobs}")
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/nautobot/extras/plugins/utils.py", line 46, in import_object
    spec.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/source/nautobot_fallout/jobs.py", line 10, in <module>
    from nautobot_golden_config.models import GoldenConfig
  File "/usr/local/lib/python3.11/site-packages/nautobot_golden_config/models.py", line 20, in <module>
    from nautobot_golden_config.utilities.constant import ENABLE_SOTAGG, PLUGIN_CFG
  File "/usr/local/lib/python3.11/site-packages/nautobot_golden_config/utilities/constant.py", line 14, in <module>
    DEFAULT_DEPLOY_STATUS = PLUGIN_CFG["default_deploy_status"]
                            ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^

Each of these would be solved by just using a get(). This would also affect users who are setting up the App and don't have their settings exactly as expected. Just like general exception handling I'd expect a check for the setting existing and not assuming it's there and throwing an Exception or log if missing.

itdependsnetworks commented 5 months ago

Each of these would be solved by just using a get(). I don't see how. It wouldn't fail in the same way, but it would fail later. The signature presumes that this data is there.

For the the specific case of default_deploy_status, I may have some confusion, imho the default value should return there. Just to confirm, in order to create that error, you have a PLUGIN_CFG but not a default_deploy_status, correct?

jdrew82 commented 5 months ago

Each of these would be solved by just using a get(). I don't see how. It wouldn't fail in the same way, but it would fail later. The signature presumes that this data is there.

For the the specific case of default_deploy_status, I may have some confusion, imho the default value should return there. Just to confirm, in order to create that error, you have a PLUGIN_CFG but not a default_deploy_status, correct?

To recreate you just need a Nautobot App with a Golden Config dependency and not have nautobot_golden_config key or the remaining dictionary keys for that App in the PLUGINS_CONFIG section. In my case, I'm not using Golden Config outside of needing access to one of the objects it creates. I don't need it enabled outside of testing.

itdependsnetworks commented 5 months ago

I am not able to re-produce starting the application off using a standard invoke debug. I have tried the following

This may be similar to what we have to do in pylint (which I know you got bit on today :) ) in that we have to start the app in order to access any values.

jdrew82 commented 5 months ago

It could be that it was happening during the unit testing phase and activated due to the import of a model from Golden Config.