[generic-config-updater] Logging change just before applying it

[ghooo]

What I did

Logging a json change just before applying it. This help knowing at which step we failed change application.

How I did it

Added new log msg

Previous command output (if the output of a command-line utility has changed)

admin@vlab-01:~$ sudo config apply-patch empty-a-table.json-patch 
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "replace", "path": "/DEVICE_METADATA", "value": {}}]
Patch Applier: Validating patch is not making changes to tables without YANG models.
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating target config according to YANG models.
... [sonic-yang-mgmt logs]
Patch Applier: Sorting patch updates.
... [sonic-yang-mgmt logs]
Patch Applier: The patch was sorted into 11 changes:
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/bgp_asn"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/buffer_model"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/default_bgp_status"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/default_pfcwd_status"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/deployment_id"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/docker_routing_config_mode"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/hostname"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/hwsku"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/mac"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/platform"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost"}]
Patch Applier: Applying changes in order.
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Failed to apply patch
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH
Try "config apply-patch -h" for help.

Error: After applying patch to config, there are still some parts not updated
admin@vlab-01:~$ 

New command output (if the output of a command-line utility has changed)

admin@vlab-01:~$ sudo config apply-patch empty-a-table.json-patch 
Patch Applier: Patch application starting.
Patch Applier: Patch: [{"op": "replace", "path": "/DEVICE_METADATA", "value": {}}]
Patch Applier: Validating patch is not making changes to tables without YANG models.
Patch Applier: Getting current config db.
Patch Applier: Simulating the target full config after applying the patch.
Patch Applier: Validating target config according to YANG models.
... [sonic-yang-mgmt logs]
Patch Applier: Sorting patch updates.
... [sonic-yang-mgmt logs]
Patch Applier: The patch was sorted into 11 changes:
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/bgp_asn"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/buffer_model"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/default_bgp_status"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/default_pfcwd_status"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/deployment_id"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/docker_routing_config_mode"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/hostname"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/hwsku"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/mac"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/platform"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost"}]
Patch Applier: Applying 11 changes in order:
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/bgp_asn"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/buffer_model"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/default_bgp_status"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/default_pfcwd_status"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/deployment_id"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/docker_routing_config_mode"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/hostname"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/hwsku"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/mac"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost/platform"}]
Patch Applier:   * [{"op": "remove", "path": "/DEVICE_METADATA/localhost"}]
Patch Applier: Verifying patch updates are reflected on ConfigDB.
Failed to apply patch
Usage: config apply-patch [OPTIONS] PATCH_FILE_PATH
Try "config apply-patch -h" for help.

Error: After applying patch to config, there are still some parts not updated
admin@vlab-01:~$ 

[qiluo-msft]

To make it simple, how about

            self.logger.log_notice(f"Applying {changes_len} change(s) in order:")

And no need to use if-else.

[ghooo]

I prefer log messages to be very informative even if they need a little extra code.

In case of 0, I prefer printing a clear message saying

No changes to apply.

In case of 1 change:

Applying 1 change in order:

In case of many changes:

Applying x changes in order:

I think this makes the logs easier to read.

[ghooo]

I can make it

self.logger.log_notice(f"Applying {changes_len} change{'s' if changes_len != 1 else ''} in order{':' if changes_len > 0 else '.'}")

less informative but not by much, but still easy to understand, and consistent with the previous list.

[ghooo]

updated

[qiluo-msft]

Minor suggestion