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

Avoid NoReverseMatch exception during app startup if navigation has a bad link #789

Merged
merged 1 commit into from
Aug 10, 2021

Conversation

glennmatthews
Copy link
Contributor

Fixes: #N/A

I developed this fix while investigating #785 - while it doesn't actually address that issue (see #788 for that) I think it's still useful.

If a navigation.py (core app or plugin-defined) defines nav menu items with an invalid link name (one that cannot be successfully reverse()d), as implemented Nautobot startup will fail with a NoReverseMatch exception:

Traceback (most recent call last):
  File "/usr/local/bin/nautobot-server", line 5, in <module>
    main()
  File "/source/nautobot/core/cli.py", line 62, in main
    initializer=_configure_settings,  # Called after defaults
  File "/source/nautobot/core/runner/runner.py", line 266, in run_app
    management.execute_from_command_line([runner_name, command] + command_args)
  File "/usr/local/lib/python3.6/site-packages/django/core/management/__init__.py", line 401, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.6/site-packages/django/core/management/__init__.py", line 377, in execute
    django.setup()
  File "/usr/local/lib/python3.6/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "/usr/local/lib/python3.6/site-packages/django/apps/registry.py", line 122, in populate
    app_config.ready()
  File "/source/nautobot/extras/plugins/__init__.py", line 101, in ready
    register_plugin_menu_items(self.verbose_name, menu_items)
  File "/source/nautobot/extras/plugins/__init__.py", line 358, in register_plugin_menu_items
    weight=new_menu_button_weight,
  File "/source/nautobot/core/apps/__init__.py", line 286, in __init__
    reverse(link)
  File "/usr/local/lib/python3.6/site-packages/django/urls/base.py", line 87, in reverse
    return iri_to_uri(resolver._reverse_with_prefix(view, prefix, *args, **kwargs))
  File "/usr/local/lib/python3.6/site-packages/django/urls/resolvers.py", line 685, in _reverse_with_prefix
    raise NoReverseMatch(msg)
django.urls.exceptions.NoReverseMatch: Reverse for 'dummymodel_add_foo' not found. 'dummymodel_add_foo' is not a valid view function or pattern name.

Additionally it's not ideal for us to be calling reverse() during initial AppConfig loading at all, as there may be some timing or sequencing issues wherein an app is ready before all urlpatterns have been fully registered (this was my working theory on investigating #785 that led to this fix).

The solution delivered here is to remove the init-time reverse() check, and additionally handle bad URLs in the rendering of the nav-menu template, rather than failing the entire template rendering and throwing an exception to the user. If a bad URL is registered in the nav menu, the UI will now show this:

Screen Shot 2021-08-05 at 3 26 55 PM

Screen Shot 2021-08-05 at 3 27 06 PM

Copy link
Contributor

@carbonarok carbonarok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@glennmatthews glennmatthews merged commit c9391a4 into develop Aug 10, 2021
glennmatthews added a commit that referenced this pull request Aug 10, 2021
@glennmatthews glennmatthews deleted the gfm-issue-785 branch August 10, 2021 13:46
hellerve added a commit to hellerve/nautobot that referenced this pull request Aug 12, 2021
* next: (37 commits)
  Add release-notes for nautobot#229 and nautobot#472
  JobResult list view to show job Meta.name where available instead of class_path (nautobot#775)
  Bump version to 1.1.3-beta.1
  Bump version and add release date
  Add release-notes for nautobot#785 and nautobot#786
  Prioritize LoganImporter over built-in importers. See nautobot#785 (nautobot#788)
  Add release-note for nautobot#789
  Remove url reverse lookup from nav menu initialization (nautobot#789)
  Add release-note for nautobot#758
  Update job docs (nautobot#758)
  Custom Fields View (nautobot#735)
  Add release-notes for nautobot#742, nautobot#771, nautobot#773
  Adding Logging Examples (nautobot#771)
  Process NAUTOBOT_DEBUG environment var (nautobot#742)
  Expanding the prometheus metrics docs (nautobot#773)
  Add release-note for nautobot#782
  Update docs and examples to reflect invoke changes made by nautobot#584 (nautobot#782)
  Added release-notes for nautobot#723 and bump prerelease version
  Updates Powerfeed Utilization Data (nautobot#772)
  Bump version and release date.
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants