Deprecate Device.network

hyperglass/configuration/main.py

@@ -204,7 +204,7 @@ def init_ui_params(*, params: "Params", devices: "Devices") -> "UIParameters":
     return UIParameters(
         **_ui_params,
         version=__version__,
-        networks=devices.networks(params),
+        devices=devices.frontend(),
         parsed_data_fields=PARSED_RESPONSE_FIELDS,
         content={"credit": content_credit, "greeting": content_greeting},
     )

hyperglass/models/api/query.py

@@ -148,7 +148,7 @@ class Query(BaseModel):
     def validate_query_type(cls, value):
         """Ensure a requested query type exists."""
         devices = use_state("devices")
-        if any((device.has_directives(value) for device in devices.objects)):
+        if any((device.has_directives(value) for device in devices)):
             return value
 
         raise QueryTypeNotFound(name=value)
@@ -158,10 +158,8 @@ class Query(BaseModel):
         """Ensure query_location is defined."""
 
         devices = use_state("devices")
-        valid_id = value in devices.ids
-        valid_hostname = value in devices.hostnames
 
-        if not any((valid_id, valid_hostname)):
+        if not devices.valid_id_or_name(value):
             raise QueryLocationNotFound(location=value)
 
         return value

hyperglass/models/api/response.py

@@ -163,26 +163,12 @@ class Vrf(BaseModel):
         }
 
 
-class Network(BaseModel):
-    """Response model for /api/devices networks."""
-
-    name: StrictStr
-    display_name: StrictStr
-
-    class Config:
-        """Pydantic model configuration."""
-
-        title = "Network"
-        description = "Network/ASN attributes"
-        schema_extra = {"examples": [{"name": "primary", "display_name": "AS65000"}]}
-
-
 class RoutersResponse(BaseModel):
     """Response model for /api/devices list items."""
 
     id: StrictStr
     name: StrictStr
-    network: StrictStr
+    group: StrictStr
 
     class Config:
         """Pydantic model configuration."""
@@ -191,7 +177,7 @@ class RoutersResponse(BaseModel):
         description = "Device attributes"
         schema_extra = {
             "examples": [
-                {"id": "nyc_router_1", "name": "NYC Router 1", "network": "New York City, NY"}
+                {"id": "nyc_router_1", "name": "NYC Router 1", "group": "New York City, NY"}
             ]
         }
 

hyperglass/models/config/devices.py

@@ -27,9 +27,7 @@ from .ssl import Ssl
 from ..main import MultiModel, HyperglassModel, HyperglassModelWithId
 from ..util import check_legacy_fields
 from .proxy import Proxy
-from .params import Params
 from ..fields import SupportedDriver
-from .network import Network
 from ..directive import Directives
 from .credential import Credential
 
@@ -46,7 +44,7 @@ class Device(HyperglassModelWithId, extra="allow"):
     id: StrictStr
     name: StrictStr
     address: Union[IPv4Address, IPv6Address, StrictStr]
-    network: Network
+    group: Optional[StrictStr]
     credential: Credential
     proxy: Optional[Proxy]
     display_name: Optional[StrictStr]
@@ -60,7 +58,7 @@ class Device(HyperglassModelWithId, extra="allow"):
 
     def __init__(self, **kw) -> None:
         """Check legacy fields and ensure an `id` is set."""
-        kw = check_legacy_fields("Device", **kw)
+        kw = check_legacy_fields(model="Device", data=kw)
         if "id" not in kw:
             kw = self._with_id(kw)
         super().__init__(**kw)
@@ -100,7 +98,7 @@ class Device(HyperglassModelWithId, extra="allow"):
         return {
             "id": self.id,
             "name": self.name,
-            "network": self.network.display_name,
+            "group": self.group,
         }
 
     @property
@@ -269,6 +267,13 @@ class Devices(MultiModel, model=Device, unique_by="id"):
         """Export API-facing device fields."""
         return [d.export_api() for d in self]
 
+    def valid_id_or_name(self, value: str) -> bool:
+        """Determine if a value is a valid device name or ID."""
+        for device in self:
+            if value == device.id or value == device.name:
+                return True
+        return False
+
     def directive_plugins(self) -> Dict[Path, Tuple[StrictStr]]:
         """Get a mapping of plugin paths to associated directive IDs."""
         result: Dict[Path, Set[StrictStr]] = {}
@@ -286,22 +291,23 @@ class Devices(MultiModel, model=Device, unique_by="id"):
         # Convert the directive set to a tuple.
         return {k: tuple(v) for k, v in result.items()}
 
-    def networks(self, params: Params) -> List[Dict[str, Any]]:
-        """Group devices by network."""
-        names = {device.network.display_name for device in self}
+    def frontend(self) -> List[Dict[str, Any]]:
+        """Export grouped devices for UIParameters."""
+        params = use_state("params")
+        groups = {device.group for device in self}
         return [
             {
-                "display_name": name,
+                "group": group,
                 "locations": [
                     {
                         "id": device.id,
                         "name": device.name,
-                        "network": device.network.display_name,
+                        "group": group,
                         "directives": [d.frontend(params) for d in device.directives],
                     }
                     for device in self
-                    if device.network.display_name == name
+                    if device.group == group
                 ],
             }
-            for name in names
+            for group in groups
         ]

hyperglass/models/config/network.py

@@ -1,20 +0,0 @@
-"""Validate network configuration variables."""
-
-# Third Party
-from pydantic import Field, StrictStr
-
-# Local
-from ..main import HyperglassModel
-
-
-class Network(HyperglassModel):
-    """Validation Model for per-network/asn config in devices.yaml."""
-
-    name: StrictStr = Field(
-        ..., title="Network Name", description="Internal name of the device's primary network.",
-    )
-    display_name: StrictStr = Field(
-        ...,
-        title="Network Display Name",
-        description="Display name of the device's primary network.",
-    )

hyperglass/models/config/proxy.py

@@ -28,7 +28,7 @@ class Proxy(HyperglassModel):
 
     def __init__(self: "Proxy", **kwargs: Any) -> None:
         """Check for legacy fields."""
-        kwargs = check_legacy_fields("Proxy", **kwargs)
+        kwargs = check_legacy_fields(model="Proxy", data=kwargs)
         super().__init__(**kwargs)
 
     @property

hyperglass/models/tests/test_util.py

@@ -7,16 +7,24 @@ import pytest
 from ..util import check_legacy_fields
 
 
+@pytest.mark.dependency()
 def test_check_legacy_fields():
     test1 = {"name": "Device A", "nos": "juniper"}
     test1_expected = {"name": "Device A", "platform": "juniper"}
     test2 = {"name": "Device B", "platform": "juniper"}
     test3 = {"name": "Device C"}
-    assert set(check_legacy_fields("Device", **test1).keys()) == set(
+    test4 = {"name": "Device D", "network": "this is wrong"}
+
+    assert set(check_legacy_fields(model="Device", data=test1).keys()) == set(
         test1_expected.keys()
     ), "legacy field not replaced"
-    assert set(check_legacy_fields("Device", **test2).keys()) == set(
+
+    assert set(check_legacy_fields(model="Device", data=test2).keys()) == set(
         test2.keys()
     ), "new field not left unmodified"
+
     with pytest.raises(ValueError):
-        check_legacy_fields("Device", **test3)
+        check_legacy_fields(model="Device", data=test3)
+
+    with pytest.raises(ValueError):
+        check_legacy_fields(model="Device", data=test4)

hyperglass/models/ui.py

@@ -42,14 +42,14 @@ class UILocation(HyperglassModel):
 
     id: StrictStr
     name: StrictStr
-    network: StrictStr
+    group: StrictStr
     directives: List[UIDirective] = []
 
 
-class UINetwork(HyperglassModel):
-    """UI: Network."""
+class UIDevices(HyperglassModel):
+    """UI: Devices."""
 
-    display_name: StrictStr
+    group: StrictStr
     locations: List[UILocation] = []
 
 
@@ -67,6 +67,6 @@ class UIParameters(ParamsPublic, HyperglassModel):
     web: WebPublic
     messages: Messages
     version: StrictStr
-    networks: List[UINetwork] = []
+    devices: List[UIDevices] = []
     parsed_data_fields: Tuple[StructuredDataField, ...]
     content: UIContent

hyperglass/models/util.py

@@ -3,28 +3,70 @@
 # Standard Library
 from typing import Any, Dict, Tuple
 
+# Third Party
+from pydantic import BaseModel
+
 # Project
 from hyperglass.log import log
 
-LEGACY_FIELDS: Dict[str, Tuple[Tuple[str, str], ...]] = {
-    "Device": (("nos", "platform"),),
-    "Proxy": (("nos", "platform"),),
+
+class LegacyField(BaseModel):
+    """Define legacy fields on a per-model basis.
+
+    When `overwrite` is `True`, the old key is replaced with the new
+    key. This will generally only occur when the value type is the same,
+    and the key name has only changed names for clarity or cosmetic
+    purposes.
+
+    When `overwrite` is `False` and the old key is found, an error is
+    raised. This generally occurs when the overall function of the old
+    and new keys has remained the same, but the value type has changed,
+    requiring the user to make changes to the config file.
+
+    When `required` is `True` and neither the old or new keys are found,
+    an error is raised. When `required` is false and neither keys are
+    found, nothing happens.
+    """
+
+    old: str
+    new: str
+    overwrite: bool = False
+    required: bool = True
+
+
+LEGACY_FIELDS: Dict[str, Tuple[LegacyField, ...]] = {
+    "Device": (
+        LegacyField(old="nos", new="platform", overwrite=True),
+        LegacyField(old="network", new="group", overwrite=False, required=False),
+    ),
+    "Proxy": (LegacyField(old="nos", new="platform", overwrite=True),),
 }
 
 
-def check_legacy_fields(model: str, **kwargs: Dict[str, Any]) -> Dict[str, Any]:
+def check_legacy_fields(*, model: str, data: Dict[str, Any]) -> Dict[str, Any]:
     """Check for legacy fields prior to model initialization."""
     if model in LEGACY_FIELDS:
-        for legacy_key, new_key in LEGACY_FIELDS[model]:
-            legacy_value = kwargs.pop(legacy_key, None)
-            new_value = kwargs.get(new_key)
+        for field in LEGACY_FIELDS[model]:
+            legacy_value = data.pop(field.old, None)
+            new_value = data.get(field.new)
             if legacy_value is not None and new_value is None:
-                log.warning(
-                    "The {} field has been deprecated and will be removed in a future release. Use the '{}' field moving forward.",
-                    f"{model}.{legacy_key}",
-                    new_key,
-                )
-                kwargs[new_key] = legacy_value
-            elif legacy_value is None and new_value is None:
-                raise ValueError(f"'{new_key}' is missing")
-    return kwargs
+                if field.overwrite:
+                    log.warning(
+                        (
+                            "The {!r} field has been deprecated and will be removed in a future release. "
+                            "Use the {!r} field moving forward."
+                        ),
+                        f"{model}.{field.old}",
+                        field.new,
+                    )
+                    data[field.new] = legacy_value
+                else:
+                    raise ValueError(
+                        (
+                            "The {!r} field has been replaced with the {!r} field. "
+                            "Please consult the documentation and/or changelog to determine the appropriate migration path."
+                        ).format(f"{model}.{field.old}", field.new)
+                    )
+            elif legacy_value is None and new_value is None and field.required:
+                raise ValueError(f"'{field.new}' is missing")
+    return data

hyperglass/plugins/tests/test_bgp_route_juniper.py

@@ -17,7 +17,10 @@ from hyperglass.models.data.bgp_route import BGPRouteTable
 from .._builtin.bgp_route_juniper import BGPRoutePluginJuniper
 
 DEPENDS_KWARGS = {
-    "depends": ["hyperglass/external/tests/test_rpki.py::test_rpki"],
+    "depends": [
+        "hyperglass/models/tests/test_util.py::test_check_legacy_fields",
+        "hyperglass/external/tests/test_rpki.py::test_rpki",
+    ],
     "scope": "session",
 }
 
@@ -32,7 +35,7 @@ def _tester(sample: str):
     device = Device(
         name="Test Device",
         address="127.0.0.1",
-        network={"name": "Test Network", "display_name": "Test Network"},
+        group="Test Network",
         credential={"username": "", "password": ""},
         platform="juniper",
         structured_output=True,

hyperglass/ui/components/form/queryLocation.tsx

@@ -6,18 +6,18 @@ import { Select } from '~/components';
 import { useConfig, useColorValue } from '~/context';
 import { useOpposingColor, useFormState } from '~/hooks';
 
-import type { Network, SingleOption, OptionGroup, FormData } from '~/types';
+import type { DeviceGroup, SingleOption, OptionGroup, FormData } from '~/types';
 import type { TQuerySelectField, LocationCardProps } from './types';
 
-function buildOptions(networks: Network[]): OptionGroup[] {
-  return networks
-    .map(net => {
-      const label = net.displayName;
-      const options = net.locations
+function buildOptions(devices: DeviceGroup[]): OptionGroup[] {
+  return devices
+    .map(group => {
+      const label = group.group;
+      const options = group.locations
         .map(loc => ({
           label: loc.name,
           value: loc.id,
-          group: net.displayName,
+          group: loc.group,
         }))
         .sort((a, b) => (a.label < b.label ? -1 : a.label > b.label ? 1 : 0));
       return { label, options };
@@ -115,14 +115,14 @@ const LocationCard = (props: LocationCardProps): JSX.Element => {
 export const QueryLocation = (props: TQuerySelectField): JSX.Element => {
   const { onChange, label } = props;
 
-  const { networks } = useConfig();
+  const { devices } = useConfig();
   const {
     formState: { errors },
   } = useFormContext<FormData>();
   const selections = useFormState(s => s.selections);
   const setSelection = useFormState(s => s.setSelection);
   const { form, filtered } = useFormState(({ form, filtered }) => ({ form, filtered }));
-  const options = useMemo(() => buildOptions(networks), [networks]);
+  const options = useMemo(() => buildOptions(devices), [devices]);
   const element = useMemo(() => {
     const groups = options.length;
     const maxOptionsPerGroup = Math.max(...options.map(opt => opt.options.length));

hyperglass/ui/components/results/individual.tsx

@@ -7,6 +7,7 @@ import {
   chakra,
   HStack,
   Tooltip,
+  useToast,
   AccordionItem,
   AccordionPanel,
   AccordionButton,
@@ -44,6 +45,7 @@ const _Result: React.ForwardRefRenderFunction<HTMLDivElement, ResultProps> = (
   ref,
 ) => {
   const { index, queryLocation } = props;
+  const toast = useToast();
   const { web, cache, messages } = useConfig();
   const { index: indices, setIndex } = useAccordionContext();
   const getDevice = useDevice();
@@ -66,7 +68,9 @@ const _Result: React.ForwardRefRenderFunction<HTMLDivElement, ResultProps> = (
     },
     {
       onSuccess(data) {
-        addResponse(device.id, data);
+        if (device !== null) {
+          addResponse(device.id, data);
+        }
       },
       onError(error) {
         console.error(error);
@@ -150,6 +154,21 @@ const _Result: React.ForwardRefRenderFunction<HTMLDivElement, ResultProps> = (
       }
     }
   }, [data, index, indices, isLoading, isError, setIndex]);
+
+  if (device === null) {
+    const id = `toast-queryLocation-${index}-${queryLocation}`;
+    if (!toast.isActive(id)) {
+      toast({
+        id,
+        title: messages.general,
+        description: `Configuration for device with ID '${queryLocation}' not found.`,
+        status: 'error',
+        isClosable: true,
+      });
+    }
+    return <></>;
+  }
+
   return (
     <AnimatedAccordionItem
       ref={ref}

hyperglass/ui/hooks/types.ts

@@ -29,12 +29,12 @@ export interface UseGreeting {
   close(): void;
 }
 
-export type TUseDevice = (
+export type UseDevice = (
   /**
    * Device's ID, e.g. the device.name field.
    */
   deviceId: string,
-) => Device;
+) => Device | null;
 
 export type UseStrfArgs = { [k: string]: unknown } | string;
 

hyperglass/ui/hooks/useDevice.ts

@@ -2,19 +2,19 @@ import { useCallback, useMemo } from 'react';
 import { useConfig } from '~/context';
 
 import type { Device } from '~/types';
-import type { TUseDevice } from './types';
+import type { UseDevice } from './types';
 
 /**
  * Get a device's configuration from the global configuration context based on its name.
  */
-export function useDevice(): TUseDevice {
-  const { networks } = useConfig();
+export function useDevice(): UseDevice {
+  const { devices } = useConfig();
 
-  const devices = useMemo(() => networks.map(n => n.locations).flat(), [networks]);
+  const locations = useMemo(() => devices.map(group => group.locations).flat(), [devices]);
 
-  function getDevice(id: string): Device {
-    return devices.filter(dev => dev.id === id)[0];
+  function getDevice(id: string): Device | null {
+    return locations.find(device => device.id === id) ?? null;
   }
 
-  return useCallback(getDevice, [devices]);
+  return useCallback(getDevice, [locations]);
 }

hyperglass/ui/hooks/useFormState.ts

@@ -8,7 +8,7 @@ import { all, andJoin, dedupObjectArray, withDev } from '~/util';
 import type { StateCreator } from 'zustand';
 import type { UseFormSetError, UseFormClearErrors } from 'react-hook-form';
 import type { SingleOption, Directive, FormData, Text } from '~/types';
-import type { TUseDevice } from './types';
+import type { UseDevice } from './types';
 
 type FormStatus = 'form' | 'results';
 
@@ -67,7 +67,7 @@ interface FormStateType {
     extra: {
       setError: UseFormSetError<FormData>;
       clearErrors: UseFormClearErrors<FormData>;
-      getDevice: TUseDevice;
+      getDevice: UseDevice;
       text: Text;
     },
   ): void;
@@ -129,7 +129,7 @@ const formState: StateCreator<FormStateType> = (set, get) => ({
     extra: {
       setError: UseFormSetError<FormData>;
       clearErrors: UseFormClearErrors<FormData>;
-      getDevice: TUseDevice;
+      getDevice: UseDevice;
       text: Text;
     },
   ): void {
@@ -144,15 +144,17 @@ const formState: StateCreator<FormStateType> = (set, get) => ({
 
     for (const loc of locations) {
       const device = getDevice(loc);
-      locationNames.push(device.name);
-      allDevices.push(device);
-      const groups = new Set<string>();
-      for (const directive of device.directives) {
-        for (const group of directive.groups) {
-          groups.add(group);
+      if (device !== null) {
+        locationNames.push(device.name);
+        allDevices.push(device);
+        const groups = new Set<string>();
+        for (const directive of device.directives) {
+          for (const group of directive.groups) {
+            groups.add(group);
+          }
         }
+        allGroups.push(Array.from(groups));
       }
-      allGroups.push(Array.from(groups));
     }
 
     const intersecting = intersectionWith(...allGroups, isEqual);

hyperglass/ui/types/config.ts

@@ -97,36 +97,6 @@ interface _Web {
   theme: _ThemeConfig;
 }
 
-// export interface Query {
-//   name: string;
-//   enable: boolean;
-//   display_name: string;
-// }
-
-// export interface BGPCommunity {
-//   community: string;
-//   display_name: string;
-//   description: string;
-// }
-
-// export interface QueryBGPRoute extends Query {}
-// export interface QueryBGPASPath extends Query {}
-// export interface QueryPing extends Query {}
-// export interface QueryTraceroute extends Query {}
-// export interface QueryBGPCommunity extends Query {
-//   mode: 'input' | 'select';
-//   communities: BGPCommunity[];
-// }
-
-// export interface Queries {
-//   bgp_route: QueryBGPRoute;
-//   bgp_community: QueryBGPCommunity;
-//   bgp_aspath: QueryBGPASPath;
-//   ping: QueryPing;
-//   traceroute: QueryTraceroute;
-//   list: Query[];
-// }
-
 type _DirectiveBase = {
   id: string;
   name: string;
@@ -151,15 +121,10 @@ type _Directive = _DirectiveBase | _DirectiveSelect;
 interface _Device {
   id: string;
   name: string;
-  network: string;
+  group: string;
   directives: _Directive[];
 }
 
-interface _Network {
-  display_name: string;
-  locations: _Device[];
-}
-
 interface _QueryContent {
   content: string;
   enable: boolean;
@@ -184,13 +149,16 @@ interface _Cache {
 
 type _Config = _ConfigDeep & _ConfigShallow;
 
+interface _DeviceGroup {
+  group: string;
+  locations: _Device[];
+}
+
 interface _ConfigDeep {
   cache: _Cache;
   web: _Web;
   messages: _Messages;
-  // queries: Queries;
-  devices: _Device[];
-  networks: _Network[];
+  devices: _DeviceGroup[];
   content: _Content;
 }
 
@@ -225,8 +193,8 @@ export type Config = CamelCasedPropertiesDeep<_ConfigDeep> & CamelCasedPropertie
 export type ThemeConfig = CamelCasedProperties<_ThemeConfig>;
 export type Content = CamelCasedProperties<_Content>;
 export type QueryContent = CamelCasedPropertiesDeep<_QueryContent>;
-export type Network = CamelCasedPropertiesDeep<_Network>;
 export type Device = CamelCasedPropertiesDeep<_Device>;
+export type DeviceGroup = CamelCasedPropertiesDeep<_DeviceGroup>;
 export type Directive = CamelCasedPropertiesDeep<_Directive>;
 export type DirectiveSelect = CamelCasedPropertiesDeep<_DirectiveSelect>;
 export type DirectiveOption = CamelCasedPropertiesDeep<_DirectiveOption>;