address PR comments

config/main.py

@@ -102,7 +102,7 @@
 TTL_RANGE = click.IntRange(min=0, max=255)
 QUEUE_RANGE = click.IntRange(min=0, max=255)
 GRE_TYPE_RANGE = click.IntRange(min=0, max=65535)
-ADHOC_VALIDATION = False
+ADHOC_VALIDATION = True
 
 # Load sonic-cfggen from source since /usr/local/bin/sonic-cfggen does not have .py extension.
 sonic_cfggen = load_module_from_source('sonic_cfggen', '/usr/local/bin/sonic-cfggen')
@@ -1898,7 +1898,7 @@ def add_portchannel(ctx, portchannel_name, min_links, fallback):
 
     if ADHOC_VALIDATION:
         if is_portchannel_name_valid(portchannel_name) != True:
-            ctx.fail("{} is invalid!, name should have prefix '{}' and sufix '{}'"
+            ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'"
                     .format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO))
 
     db = ValidatedConfigDBConnector(ctx.obj['db'], ADHOC_VALIDATION) 

config/validated_config_db_connector.py

@@ -2,10 +2,13 @@
 from jsonpointer import JsonPointer
 
 from generic_config_updater.generic_updater import GenericUpdater, ConfigFormat
+from generic_config_updater.gu_common import EmptyTableError
+from swsscommon.swsscommon import ConfigDBConnector
 
 def ValidatedConfigDBConnector(config_db_connector, fallback_adhoc_validation):
     if not fallback_adhoc_validation:
         config_db_connector.set_entry = validated_set_entry
+        config_db_connector.mod_entry = validated_mod_entry
     return config_db_connector
 
 def make_path_value_jsonpatch_compatible(table, key, value):
@@ -30,6 +33,29 @@ def validated_set_entry(table, key, value):
     if op == "add":
         gcu_json["value"] = value
 
+    gcu_json_input.append(gcu_json)
+    gcu_patch = jsonpatch.JsonPatch(gcu_json_input)
+    format = ConfigFormat.CONFIGDB.name
+    config_format = ConfigFormat[format.upper()]
+    try:
+        GenericUpdater().apply_patch(patch=gcu_patch, config_format=config_format, verbose=False, dry_run=False, ignore_non_yang_tables=False, ignore_paths=None)
+    except EmptyTableError:
+        config_db = ConfigDBConnector()
+        config_db.connect()
+        config_db.set_entry(table, key, None)
+
+def validated_mod_entry(table, key, value):
+    if value is not None:
+        op = "add"
+    else:
+        op = "remove"
+    path = "/{}/{}/{}".format(table, key, value[0])
+    gcu_json_input = []
+    gcu_json = {"op": "{}".format(op),
+                "path": "{}".format(path)}
+    if op == "add": 
+        gcu_json["value"] = value[1]
+
     gcu_json_input.append(gcu_json)
     gcu_patch = jsonpatch.JsonPatch(gcu_json_input)
     format = ConfigFormat.CONFIGDB.name

config/vlan.py

@@ -35,7 +35,7 @@ def add_vlan(db, vid):
         ctx.fail("{} is default VLAN".format(vlan))
 
     if ADHOC_VALIDATION:
-        if clicommon.check_if_vlanid_exist(db.cfgdb, vlan): # MISSING CONSTRAINT IN YANG MODEL
+        if clicommon.check_if_vlanid_exist(db.cfgdb, vlan): # TODO: MISSING CONSTRAINT IN YANG MODEL
             ctx.fail("{} already exists".format(vlan))
 
     config_db = ValidatedConfigDBCOnnector(db.cfgdb, ADHOC_VALIDATION)
@@ -64,24 +64,20 @@ def del_vlan(db, vid):
 
         intf_table = db.cfgdb.get_table('VLAN_INTERFACE')
         for intf_key in intf_table:
-            if ((type(intf_key) is str and intf_key == 'Vlan{}'.format(vid)) or # MISSING CONSTRAINT IN YANG MODEL
+            if ((type(intf_key) is str and intf_key == 'Vlan{}'.format(vid)) or # TODO: MISSING CONSTRAINT IN YANG MODEL
                 (type(intf_key) is tuple and intf_key[0] == 'Vlan{}'.format(vid))):
                 ctx.fail("{} can not be removed. First remove IP addresses assigned to this VLAN".format(vlan))
 
         keys = [ (k, v) for k, v in db.cfgdb.get_table('VLAN_MEMBER') if k == 'Vlan{}'.format(vid) ]
 
-        if keys: # MISSING CONSTRAINT IN YANG MODEL
+        if keys: # TODO: MISSING CONSTRAINT IN YANG MODEL
             ctx.fail("VLAN ID {} can not be removed. First remove all members assigned to this VLAN.".format(vid))
 
     config_db = ValidatedConfigDBConnector(db.cfgdb, ADHOC_VALIDATION)
     try: 
         config_db.set_entry('VLAN', 'Vlan{}'.format(vid), None)
     except JsonPatchConflict:
         ctx.fail("{} does not exist".format(vlan))
-    except ValueError: # GCU prohibits empty table, but it is otherwise allowed. So we temporarily catch GCU error and then directly set empty tabe 
-        config_db = ConfigDBConnector()
-        config_db.connect()
-        config_db.set_entry('VLAN', 'Vlan{}'.format(vid), None)
 
 def restart_ndppd():
     verify_swss_running_cmd = "docker container inspect -f '{{.State.Status}}' swss"
@@ -116,7 +112,8 @@ def config_proxy_arp(db, vid, mode):
     if not clicommon.is_valid_vlan_interface(db.cfgdb, vlan):
         ctx.fail("Interface {} does not exist".format(vlan))
 
-    db.cfgdb.mod_entry('VLAN_INTERFACE', vlan, {"proxy_arp": mode})
+    config_db = ValidatedConfigDBConnector(db.cfgdb)
+    config_db.mod_entry('VLAN_INTERFACE', vlan, {"proxy_arp": mode})
     click.echo('Proxy ARP setting saved to ConfigDB')
     restart_ndppd()
 #

generic_config_updater/generic_updater.py

@@ -1,7 +1,7 @@
 import json
 import os
 from enum import Enum
-from .gu_common import GenericConfigUpdaterError, ConfigWrapper, \
+from .gu_common import GenericConfigUpdaterError, EmptyTableError, ConfigWrapper, \
                        DryRunConfigWrapper, PatchWrapper, genericUpdaterLogging
 from .patch_sorter import StrictPatchSorter, NonStrictPatchSorter, ConfigSplitter, \
                           TablesWithoutYangConfigSplitter, IgnorePathsFromYangConfigSplitter
@@ -54,7 +54,7 @@ def apply(self, patch):
         empty_tables = self.config_wrapper.get_empty_tables(target_config)
         if empty_tables: # if there are empty tables
             empty_tables_txt = ", ".join(empty_tables)
-            raise ValueError("Given patch is not valid because it will result in empty tables " \
+            raise EmptyTableError("Given patch is not valid because it will result in empty tables " \
                              "which is not allowed in ConfigDb. " \
                             f"Table{'s' if len(empty_tables) != 1 else ''}: {empty_tables_txt}")
 

generic_config_updater/gu_common.py

@@ -16,6 +16,9 @@
 class GenericConfigUpdaterError(Exception):
     pass
 
+class EmptyTableError(Exception):
+    pass
+
 class JsonChange:
     """
     A class that describes a partial change to a JSON object.