nautobot / nautobot-app-ssot

Single Source of Truth for Nautobot
https://docs.nautobot.com/projects/ssot/en/latest/
Other
38 stars 38 forks source link

Jobs import confusing message #556

Open itdependsnetworks opened 1 month ago

itdependsnetworks commented 1 month ago

Environment

Expected Behavior

I am not exactly sure, and perhaps not really a bug, but jobs do not register when you are missing sub-dependencies.

Observed Behavior

>>> import_module("nautobot_ssot.integrations.infoblox.jobs")
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "/usr/local/lib/python3.11/importlib/__init__.py", line 126, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<frozen importlib._bootstrap>", line 1204, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1176, in _find_and_load
  File "<frozen importlib._bootstrap>", line 1147, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 690, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 940, in exec_module
  File "<frozen importlib._bootstrap>", line 241, in _call_with_frames_removed
  File "/usr/local/lib/python3.11/site-packages/nautobot_ssot/integrations/infoblox/jobs.py", line 12, in <module>
    from .diffsync.adapters import infoblox, nautobot
  File "/usr/local/lib/python3.11/site-packages/nautobot_ssot/integrations/infoblox/diffsync/adapters/infoblox.py", line 22, in <module>
    from nautobot_ssot.integrations.infoblox.utils.client import get_default_ext_attrs
  File "/usr/local/lib/python3.11/site-packages/nautobot_ssot/integrations/infoblox/utils/client.py", line 15, in <module>
    from dns import reversename
ModuleNotFoundError: No module named 'dns'

Steps to Reproduce

  1. don't install dnspython
  2. enable infoblox
  3. run nautobot-server migrate to have jobs populate
jdrew82 commented 1 month ago

This is expected as we make it quite clear that you must install dependencies for an integration in order for it to be functional, including the Jobs.

itdependsnetworks commented 4 weeks ago

I won't fall on my sword on it, but ideally we can separate these two so that we know what issue is actually happening. From a POLA perspective, I expect to fail early if I don't have the right dependencies.

lampwins commented 4 weeks ago

The typical pythonic way of doing this is to wrap the import in a try/except to catch the ImportError and raise a more meaningful error. Here is an example in django-tables2 https://github.com/jieter/django-tables2/blob/master/django_tables2/export/export.py#L4-L9

itdependsnetworks commented 4 weeks ago

The challenge we have is there are two different reasons to have an ImportError

  1. There is no job in the integration, therefor it should be skipped (this is what the code is intended to do)
  2. You did not provide the correct dependencies, I would think this should fail, but is silently skipped so it is difficult to understand the error.

In thinking about it, I think we should just move to a manual definition of the job to integration mapping, and just let ImportError's do their thing. This will cause an additional step to the developer, but they would not get far without doing this step. So it would be less empathetic to the developer, but more empathetic to the admins.