nautobot-app-ssot icon indicating copy to clipboard operation
nautobot-app-ssot copied to clipboard

Dry run does not execute `execute_sync`, therefore doesn't tests create/update/delete operations

Open Kircheneer opened this issue 1 year ago • 5 comments

Environment

  • Python version: 3.8.17
  • Nautobot version: 1.5.24
  • nautobot-ssot version: 1.1.2

Expected Behavior

An SSoT job to report errors during model validation (i.e. validated_save()) when I run an SSoT job with the dry run option checked.

Observed Behavior

execute_sync is not executed when the dry run option is checked, therefore model validation issues aren't reported in a dry run.

Steps to Reproduce

  1. Create a job that purposefully has validation errors, such as a missing foreign key or required field
  2. Run the job with dry run set
  3. Observe no errors

The problem lies here: https://github.com/nautobot/nautobot-plugin-ssot/blob/develop/nautobot_ssot/jobs/base.py#L169

Kircheneer avatar Aug 07 '23 13:08 Kircheneer

This interacts with the topic of atomic transactions and those longer being automatically defaulted to in 2.0 as we may need to implement some custom logic there to enable the possibility of dry running.

Thoughts on different implementation possibilities:

  • dry run functionality directly in diffsync, i.e. introduce a global flag that is up to the user to implement and conveys "dry run/don't commit result"
  • on dry run, wrap execute_sync in an automatic atomic transaction and roll that back at the end

There's probably more options, but those are the two I can think of right now

Kircheneer avatar Aug 07 '23 13:08 Kircheneer

this idea of the atomic transaction makes sense when you have Nautobot on the target side, but not generally speaking, for other external API targets. This was the reason to make the dry-run equivalent to the diffsync diff output.

chadell avatar Aug 16 '23 13:08 chadell

related: nautobot/nautobot-plugin-ssot#159

glennmatthews avatar Aug 17 '23 17:08 glennmatthews

I think we need to discuss how much of this you're expecting to have validated. With the changes made in 2.0 I'm not sure it makes sense to add this feature. When you're running a dry-run in this case aren't you more concerned with the diff output and not necessarily what the results are when perform CRUD in Nautobot?

jdrew82 avatar Nov 27 '23 22:11 jdrew82

My concern here is that we don't have model validation, i.e. the dry run may succeed but the actual run will fail, because the validation has failed. I guess this is only possible when inside an atomic transaction.

Kircheneer avatar Nov 28 '23 08:11 Kircheneer