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

Fix constance being accessed before database is ready and other fixes #341

Merged
merged 9 commits into from
Nov 15, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/341.added
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added a "grafana disabled" view in case a user clicks on a grafana nav menu item when the grafana integration is disabled.
4 changes: 4 additions & 0 deletions changes/341.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Fixed django-constance not being upgradable due to this app accessing the database before migrations could run.
Removed conditional logic for adding grafana navigation menu items.
Fixed Nautobot v2.3 incompatibility caused by saved views not being able to determine the models' table classes.
Added exception handling for cases where diffsync is not installed, since it's marked as optional.
1 change: 1 addition & 0 deletions changes/341.housekeeping
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixed dev environment nautobot_config.py to fall back to constance if environment variable is not used.
glennmatthews marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions changes/341.removed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Removed all grafana integration API files since there we no API views provided by grafana integration.
47 changes: 12 additions & 35 deletions nautobot_chatops/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,8 @@

import logging

from django.urls import include, path
from django.urls import path
from nautobot.apps.api import OrderedDefaultRouter
from nautobot.apps.config import get_app_settings_or_config

from nautobot_chatops.api.views.generic import (
AccessGrantViewSet,
Expand All @@ -13,44 +12,24 @@
NautobotChatopsRootView,
)
from nautobot_chatops.api.views.lookup import AccessLookupView, UserEmailLookupView
from nautobot_chatops.api.views.mattermost import MattermostInteractionView, MattermostSlashCommandView
from nautobot_chatops.api.views.ms_teams import MSTeamsMessagesView
from nautobot_chatops.api.views.slack import SlackEventAPIView, SlackInteractionView, SlackSlashCommandView
from nautobot_chatops.api.views.webex import WebexView

logger = logging.getLogger(__name__)
urlpatterns = [
path("lookup/", AccessLookupView.as_view(), name="access_lookup"),
path("email-lookup/", UserEmailLookupView.as_view(), name="email_lookup"),
path("slack/slash_command/", SlackSlashCommandView.as_view(), name="slack_slash_command"),
path("slack/interaction/", SlackInteractionView.as_view(), name="slack_interaction"),
path("slack/event/", SlackEventAPIView.as_view(), name="slack_event"),
path("ms_teams/messages/", MSTeamsMessagesView.as_view(), name="ms_teams_messages"),
path("webex/", WebexView.as_view(), name="webex"),
path("mattermost/slash_command/", MattermostSlashCommandView.as_view(), name="mattermost_slash_command"),
path("mattermost/interaction/", MattermostInteractionView.as_view(), name="mattermost_interaction"),
]

if get_app_settings_or_config("nautobot_chatops", "enable_slack"):
from nautobot_chatops.api.views.slack import SlackEventAPIView, SlackInteractionView, SlackSlashCommandView

urlpatterns += [
path("slack/slash_command/", SlackSlashCommandView.as_view(), name="slack_slash_command"),
path("slack/interaction/", SlackInteractionView.as_view(), name="slack_interaction"),
path("slack/event/", SlackEventAPIView.as_view(), name="slack_event"),
]

if get_app_settings_or_config("nautobot_chatops", "enable_ms_teams"):
from nautobot_chatops.api.views.ms_teams import MSTeamsMessagesView

urlpatterns += [
path("ms_teams/messages/", MSTeamsMessagesView.as_view(), name="ms_teams_messages"),
]

if get_app_settings_or_config("nautobot_chatops", "enable_webex"):
from nautobot_chatops.api.views.webex import WebexView

urlpatterns += [
path("webex/", WebexView.as_view(), name="webex"),
]

if get_app_settings_or_config("nautobot_chatops", "enable_mattermost"):
from nautobot_chatops.api.views.mattermost import MattermostInteractionView, MattermostSlashCommandView

urlpatterns += [
path("mattermost/slash_command/", MattermostSlashCommandView.as_view(), name="mattermost_slash_command"),
path("mattermost/interaction/", MattermostInteractionView.as_view(), name="mattermost_interaction"),
]

router = OrderedDefaultRouter()
router.APIRootView = NautobotChatopsRootView
router.register("commandtoken", CommandTokenViewSet)
Expand All @@ -60,5 +39,3 @@
app_name = "nautobot_chatops-api"

urlpatterns += router.urls

urlpatterns += [path("grafana/", include("nautobot_chatops.integrations.grafana.api.urls"))]
11 changes: 9 additions & 2 deletions nautobot_chatops/api/views/mattermost.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from nautobot_chatops.metrics import signature_error_cntr
from nautobot_chatops.models import CommandToken
from nautobot_chatops.utils import check_and_enqueue_command
from nautobot_chatops.views import SettingsControlledViewMixin
from nautobot_chatops.workers import commands_help, get_commands_registry, parse_command_string

# pylint: disable=logging-fstring-interpolation
Expand Down Expand Up @@ -67,8 +68,14 @@ def verify_signature(request):
return True, "Signature is valid"


class MattermostView(SettingsControlledViewMixin, View):
"""Base class for Mattermost views."""

enable_view_setting = "enable_mattermost"


@method_decorator(csrf_exempt, name="dispatch")
class MattermostSlashCommandView(View):
class MattermostSlashCommandView(MattermostView):
"""Handle notifications from a Mattermost /command."""

http_method_names = ["post"]
Expand Down Expand Up @@ -117,7 +124,7 @@ def post(self, request, *args, **kwargs):


@method_decorator(csrf_exempt, name="dispatch")
class MattermostInteractionView(View):
class MattermostInteractionView(MattermostView):
"""Handle notifications resulting from a Mattermost interactive block."""

http_method_names = ["post"]
Expand Down
4 changes: 3 additions & 1 deletion nautobot_chatops/api/views/ms_teams.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

from nautobot_chatops.dispatchers.ms_teams import MSTeamsDispatcher
from nautobot_chatops.utils import check_and_enqueue_command
from nautobot_chatops.views import SettingsControlledViewMixin
from nautobot_chatops.workers import commands_help, get_commands_registry, parse_command_string

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -113,9 +114,10 @@ def verify_jwt_token(request_headers, request_json):


@method_decorator(csrf_exempt, name="dispatch")
class MSTeamsMessagesView(View):
class MSTeamsMessagesView(SettingsControlledViewMixin, View):
"""Handle notifications from a Microsoft Teams bot."""

enable_view_setting = "enable_ms_teams"
http_method_names = ["post"]

# pylint: disable=too-many-locals,too-many-branches,too-many-statements
Expand Down
13 changes: 10 additions & 3 deletions nautobot_chatops/api/views/slack.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from nautobot_chatops.dispatchers.slack import SlackDispatcher
from nautobot_chatops.metrics import signature_error_cntr
from nautobot_chatops.utils import check_and_enqueue_command
from nautobot_chatops.views import SettingsControlledViewMixin
from nautobot_chatops.workers import commands_help, get_commands_registry, parse_command_string

# pylint: disable=logging-fstring-interpolation
Expand Down Expand Up @@ -65,8 +66,14 @@ def verify_signature(request):
return True, "Signature is valid"


class SlackView(SettingsControlledViewMixin, View):
"""Base class for Slack views."""

enable_view_setting = "enable_slack"


@method_decorator(csrf_exempt, name="dispatch")
class SlackSlashCommandView(View):
class SlackSlashCommandView(SlackView):
"""Handle notifications from a Slack /command."""

http_method_names = ["post"]
Expand Down Expand Up @@ -115,7 +122,7 @@ def post(self, request, *args, **kwargs):


@method_decorator(csrf_exempt, name="dispatch")
class SlackInteractionView(View):
class SlackInteractionView(SlackView):
"""Handle notifications resulting from a Slack interactive block or modal."""

http_method_names = ["post"]
Expand Down Expand Up @@ -276,7 +283,7 @@ def post(self, request, *args, **kwargs):


@method_decorator(csrf_exempt, name="dispatch")
class SlackEventAPIView(View):
class SlackEventAPIView(SlackView):
"""Handle notifications resulting from a mention of the Slack app."""

http_method_names = ["post"]
Expand Down
4 changes: 3 additions & 1 deletion nautobot_chatops/api/views/webex.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

from nautobot_chatops.dispatchers.webex import WEBEX_CONFIG, WebexDispatcher
from nautobot_chatops.utils import check_and_enqueue_command
from nautobot_chatops.views import SettingsControlledViewMixin
from nautobot_chatops.workers import commands_help, get_commands_registry, parse_command_string

logger = logging.getLogger(__name__)
Expand Down Expand Up @@ -76,9 +77,10 @@ def verify_signature(request):


@method_decorator(csrf_exempt, name="dispatch")
class WebexView(View):
class WebexView(SettingsControlledViewMixin, View):
"""Handle all supported inbound notifications from Webex."""

enable_view_setting = "enable_webex"
http_method_names = ["post"]

# pylint: disable=too-many-locals,too-many-return-statements,too-many-branches
Expand Down
1 change: 0 additions & 1 deletion nautobot_chatops/integrations/grafana/api/__init__.py

This file was deleted.

15 changes: 0 additions & 15 deletions nautobot_chatops/integrations/grafana/api/urls.py

This file was deleted.

5 changes: 0 additions & 5 deletions nautobot_chatops/integrations/grafana/api/views/__init__.py

This file was deleted.

11 changes: 0 additions & 11 deletions nautobot_chatops/integrations/grafana/api/views/generic.py

This file was deleted.

44 changes: 34 additions & 10 deletions nautobot_chatops/integrations/grafana/diffsync/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,22 @@

from typing import Union

from diffsync import DiffSyncFlags

from nautobot_chatops.integrations.grafana.diffsync.models import (
GrafanaDashboard,
GrafanaPanel,
GrafanaVariable,
NautobotDashboard,
NautobotPanel,
NautobotVariable,
)
try:
from diffsync import DiffSyncFlags

DIFFSYNC_INSTALLED = True
except ImportError:
DIFFSYNC_INSTALLED = False

if DIFFSYNC_INSTALLED:
from nautobot_chatops.integrations.grafana.diffsync.models import (
GrafanaDashboard,
GrafanaPanel,
GrafanaVariable,
NautobotDashboard,
NautobotPanel,
NautobotVariable,
)
from nautobot_chatops.integrations.grafana.grafana import handler
from nautobot_chatops.integrations.grafana.models import Dashboard, Panel, PanelVariable

Expand All @@ -21,7 +27,13 @@ def run_dashboard_sync(overwrite: bool = False) -> Union[str, None]:

Args:
overwrite (bool): Overwrite Nautobot data and delete records that are no longer in Grafana.

Raises:
ImportError: If DiffSync is not installed.
"""
if not DIFFSYNC_INSTALLED:
raise ImportError("DiffSync is not installed. Please install DiffSync to use the grafana integration.")
Copy link
Contributor

Choose a reason for hiding this comment

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

ImportError for these calls feels weird to me - shouldn't it be something like RuntimeError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going to use RuntimeError but then I decided to replicate the exception you would get if you tried to import diffsync in this function. It seemed more appropriate to use ImportError here but I'm happy to change it to RuntimeError if you think that's better here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be my preference, but I don't feel strongly about it.

Might be good either way to add a Raises: section to the docstring documenting this behavior?


df_flags = DiffSyncFlags.NONE if overwrite else DiffSyncFlags.SKIP_UNMATCHED_DST

# Fetch dashboards from the Grafana API
Expand Down Expand Up @@ -51,7 +63,13 @@ def run_panels_sync(dashboard: Dashboard, overwrite: bool = False) -> Union[str,
Args:
dashboard (nautobot_chatops.integrations.grafana.models.Dashboard): The dashboard we are going to do a diffsync with.
overwrite (bool): Overwrite Nautobot data and delete records that are no longer in Grafana.

Raises:
ImportError: If DiffSync is not installed.
"""
if not DIFFSYNC_INSTALLED:
raise ImportError("DiffSync is not installed. Please install DiffSync to use the grafana integration.")

df_flags = DiffSyncFlags.NONE if overwrite else DiffSyncFlags.SKIP_UNMATCHED_DST

# Fetch panels from the Grafana API
Expand Down Expand Up @@ -81,7 +99,13 @@ def run_variables_sync(dashboard: Dashboard, overwrite: bool = False) -> Union[s
Args:
dashboard (nautobot_chatops.integrations.grafana.models.Dashboard): The dashboard we are going to do a diffsync with.
overwrite (bool): Overwrite Nautobot data and delete records that are no longer in Grafana.

Raises:
ImportError: If DiffSync is not installed.
"""
if not DIFFSYNC_INSTALLED:
raise ImportError("DiffSync is not installed. Please install DiffSync to use the grafana integration.")

df_flags = DiffSyncFlags.NONE if overwrite else DiffSyncFlags.SKIP_UNMATCHED_DST

# Fetch panels from the Grafana API
Expand Down
6 changes: 3 additions & 3 deletions nautobot_chatops/integrations/grafana/navigation.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

items = [
NavMenuItem(
link="plugins:nautobot_chatops:grafanadashboards",
link="plugins:nautobot_chatops:grafanadashboard_list",
permissions=["nautobot_chatops.dashboards_read"],
name="Grafana Dashboards",
buttons=(
Expand All @@ -15,7 +15,7 @@
),
),
NavMenuItem(
link="plugins:nautobot_chatops:grafanapanel",
link="plugins:nautobot_chatops:grafanapanel_list",
permissions=["nautobot_chatops.panel_read"],
name="Grafana Panels",
buttons=(
Expand All @@ -26,7 +26,7 @@
),
),
NavMenuItem(
link="plugins:nautobot_chatops:grafanapanelvariables",
link="plugins:nautobot_chatops:grafanapanelvariable_list",
permissions=["nautobot_chatops.panelvariables_read"],
name="Grafana Variables",
buttons=(
Expand Down
Loading
Loading