-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature delices merge from develop #135
Conversation
Update the `main` branch
Release 1.0.0
This functionality is superfluous as it is already provided in the `DesignJob`. Removing transactions from the builder gives flexibility in using the builder independently from `DesignJob` and without transactions (for performance reasons).
feat: Removed `@transation.atomic` from the builder.
fix: Performance improvements
This change allows passing arguments to model `save` methods. The specific fix is for creating Git repositories where `trigger_resync=False` must be passed to the `save()` method. Fixes #39
* fix: Fixed filter lookup from new metadata class
* fix: Report rendering for Nautobot 2.0
* fix: Fixed filter lookup from new metadata class The transaction was mistakenly removed in a previous change and design errors were no longer rolling back the database. This fix corrects that error.
chore: Prep for 1.1.0 release
Version 1.1.0
Merge from main
eab1d11
to
53375c1
Compare
53375c1
to
c2eeb34
Compare
406d0eb
to
636f50d
Compare
636f50d
to
f383c94
Compare
# in the implementation | ||
previous_design = last_journal.builder_output[design_file] | ||
self.log_debug(f"Design from previous Journal: {previous_design}") | ||
# self.environment.builder_output[design_file] = copy.deepcopy(design) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is no longer relevant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, the builder_output has been completely removed from the code base as it is not required for the functionality. I will remove this commented out line.
@property | ||
def created_by(self): | ||
"""Get the username of the user who created the object.""" | ||
# TODO: if we just add a "created_by" and "last_updated_by" field, doesn't that | ||
# reduce the complexity of code that we have in the util module? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not following, what do you want to reduce?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a note for me to think about in the future. We have a property to lookup the created_by
information, but I was thinking we could just store that directly when the design is implemented instead of trying to do complex queries at a later time.
nautobot_design_builder/signals.py
Outdated
def handle_post_delete_design_instance(sender, instance, **kwargs): # pylint: disable=unused-argument | ||
"""Cleaning up the Tag created for a design instance.""" | ||
Tag.objects.get(name=f"Managed by {instance}").delete() | ||
# @receiver(signal=post_delete, sender=DesignInstance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, are we dropping the Tag per design deployment? should we leave it optional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we are dropping the automatic tagging. I have some ideas on how to implement this more generically, but I don't think that the presence of this tag is a requirement to deliver the initial design lifecycle functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a few small comments, but it LGTM!
No description provided.