Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jobs import confusing message #556

Open
itdependsnetworks opened this issue Oct 1, 2024 · 4 comments
Open

Jobs import confusing message #556

itdependsnetworks opened this issue Oct 1, 2024 · 4 comments
Labels
integration: contrib Contrib related issues and PRs status: gathering feedback Further discussion is needed to determine this issue's scope and/or implementation

Comments

@itdependsnetworks
Copy link
Contributor

itdependsnetworks commented Oct 1, 2024

Environment

  • Python version: 3.11
  • Nautobot version: 2.2.7
  • nautobot-ssot version: 2.8.1

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
Copy link
Contributor

jdrew82 commented Oct 2, 2024

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.

@jdrew82 jdrew82 added status: gathering feedback Further discussion is needed to determine this issue's scope and/or implementation integration: contrib Contrib related issues and PRs labels Oct 2, 2024
@itdependsnetworks
Copy link
Contributor Author

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
Copy link
Member

lampwins commented Oct 3, 2024

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration: contrib Contrib related issues and PRs status: gathering feedback Further discussion is needed to determine this issue's scope and/or implementation
Projects
None yet
Development

No branches or pull requests

3 participants