fix: Do not fail example data source job with invalid data #88
Conversation
nautobot_ssot/jobs/examples.py
Outdated
| @@ -436,6 +436,10 @@ def load(self): | |||
| self.job.log_debug(message=f"Loaded {region} from remote Nautobot instance") | |||
|
|
|||
| for site_entry in self._get_api_data("api/dcim/sites/"): | |||
| if not site_entry["status"]: | |||
There was a problem hiding this comment.
Thanks for the suggestions, but...
Why not apply the same logic as below:
region_name=site_entry["region"]["name"] if site_entry["region"] else None,status_slug=site_entry["status"]["value"] if site_entry["status"] else NoneThere was a problem hiding this comment.
Because status field is required. I could assign some default value there, but seems to me better to ignore (and log) invalid objects, WDYT?
There was a problem hiding this comment.
so, if it's not required any more in the Nautobot model, maybe we could update to make it optional?
https://github.com/nautobot/nautobot-plugin-ssot/blob/develop/nautobot_ssot/jobs/examples.py#L60
There was a problem hiding this comment.
Status field is required:
>>> site = Site.objects.create()
>>> site.name = "My Site"
>>> site.validated_save()
Traceback (most recent call last):
File "<console>", line 1, in <module>
File "/usr/local/lib/python3.10/site-packages/nautobot/core/models/__init__.py", line 51, in validated_save
self.full_clean()
File "/usr/local/lib/python3.10/site-packages/django/db/models/base.py", line 1251, in full_clean
raise ValidationError(errors)
django.core.exceptions.ValidationError: {'status': ['This field cannot be blank.']}
>>>
There seems to be some data inconsistency (missing status) in our demo data:
https://demo.nautobot.com/dcim/sites/hou/?tab=main
There was a problem hiding this comment.
so, if it's required we can't make it optional...
It's weird how data can be in this state if the DB schema is enforcing the status field...
How could we skip this validation and get into the DB?
There was a problem hiding this comment.
@chadell
Do you have some resolution pls?
There was a problem hiding this comment.
I think for this we should probably just be defaulting to Active Status. The problem is definitely in the demo data somehow allowing objects that require Status to be created without it blowing up the Job but we should have the Job handling it appropriately instead of just assuming the Status is set right.
There was a problem hiding this comment.
Updated to use "active" status as a fallback for missing status
There was a problem hiding this comment.
Thanks @snaselj! Does that make this ready for review?
There was a problem hiding this comment.
@jdrew82
Not yet, dry run for the source job went smoothly, but without dry run, the job get's stuck. Will check it and let you know once everything works.
snaselj
force-pushed
the
u/snaselj-fix-example-source
branch
from
216b572
to
1fc7733
Compare
|
Removed commit 216b572 as it was not working |
|
@snaselj is this ready? |
snaselj
force-pushed
the
u/snaselj-fix-example-source
branch
from
05d3293
to
dc37b85
Compare
|
Rebased to the latest develop |
|
This PR is ready for review now @Kircheneer |
About
I noticed example source job is failing now when I start with empty DB, providing PR that fixes that.
Done