Skip to content
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

Enable commiting changes after script dry-run is completed #9609

Open
jahknem opened this issue Jun 27, 2022 · 15 comments
Open

Enable commiting changes after script dry-run is completed #9609

jahknem opened this issue Jun 27, 2022 · 15 comments
Labels
complexity: low Requires minimal effort to implement status: backlog Awaiting selection for work type: feature Introduction of new functionality to the application

Comments

@jahknem
Copy link

jahknem commented Jun 27, 2022

NetBox version

v3.2.5

Feature type

New functionality

Proposed functionality

I'd like to have a commit button added to the log page of a script result. It should only appear/be clickable if the commit field has not been selcted. This commit button should add the changes to the database as though the commit field had been selected.

Use case

Currently when doing a dry run on a script, I have to uncheck the commit field. This means regardless of if the results are to my liking or not the changes do not get commited. This obviously is as intended. However it also forces the user to return to the input mask and enter their data a second time (except if it still in the browser cache, which is something that I would not want to rely on.)
My proposed change would enable users to first see the script ouput and logs before adding the the changes to the database. It is a quality of life change.

Database changes

No response

External dependencies

No response

@jahknem jahknem added the type: feature Introduction of new functionality to the application label Jun 27, 2022
@jeremystretch
Copy link
Member

I'd like to have a commit button added to the log page of a script result.

Each execution of a script is atomic. Depending on external factors that may be in play, a script which succeeds during its first run might fail immediately afterward. Thus, there is no mechanism available to "apply" the results of a script after the fact; it must be run again.

We can probably change up the workflow so that a script which is run without commit enabled yields the pre-populated form next to its results.

@jeremystretch jeremystretch added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Jun 28, 2022
@DanSheps
Copy link
Member

DanSheps commented Jul 2, 2022

I can fully get behind this, I too would like to be able to dry-run a script then when that succeeds simply re-run the form with the commit enabled.

I like the pre-population idea, however there may be other options available to us as well. Will definitely need some thought.

@jahknem
Copy link
Author

jahknem commented Jul 13, 2022

How about locking the objects touched by the script for a predefined amount of time during which the results of the script may be inspected? If it is not commited during that time the changes would be reversed.
I can think of other cases where such a locking mechanism might me useful, like when editing a device with lots of interfaces in a netbox instance where lots of people have access.

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Sep 12, 2022
@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation pending closure Requires immediate attention to avoid being closed for inactivity labels Sep 16, 2022
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Nov 16, 2022
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation pending closure Requires immediate attention to avoid being closed for inactivity labels Dec 15, 2022
@jeremystretch jeremystretch self-assigned this Dec 15, 2022
@BarbarossaTM
Copy link
Contributor

I'd also like to have this as we're starting to lean further on custom scripts to move business logic into NetBox.

I'd wished for a button like Run for real or Re-run with same values multiple times, which I guess would be the lowest hanging approach we could take (without digging very much into this). I'm open to take a stab at this if you want :-)

@jeremystretch
Copy link
Member

The scope of the work here is just to enable re-running a script with the same form values; no changes should be made to the commit toggle. @BarbarossaTM happy to assign this to you. (Please be sure to base your work off of v3.5.)

@BarbarossaTM
Copy link
Contributor

To be sure: I'd start hacking on this based on the develop branch, which I assume is what you'd want me to do @jeremystretch?

@jeremystretch
Copy link
Member

We're in a bit of transitional period at the moment: v3.4.9 will be released in the next day or two, followed very shortly by v3.5.0. If you plan to start now, you should base your work off of the feature branch, which will be merged into develop just ahead of the v3.5 release.

@jeremystretch
Copy link
Member

@BarbarossaTM you're welcome to begin work on this using the develop branch now that v3.5.0 has been released.

@BarbarossaTM
Copy link
Contributor

Thanks @jeremystretch, I've penciled this task for this week, will go for develop then.

@jeremystretch
Copy link
Member

@BarbarossaTM are you still planning to work on this?

@BarbarossaTM
Copy link
Contributor

Sorry was pretty swamped the last weeks and am doing on-call this week. I still have this on my list, if however someone were to have spare cycles right now, I'm happy to hand this over. Otherwise I'll try to prioritize this for next week.

@BarbarossaTM
Copy link
Contributor

So, finally got a chance to look into this.

From what I can gather from scripts.py we need to store the request and data somewhere, so we can re-use it on a consecutive run.

We could extend the DB model to hold that data for every script run, however that could become quite some amount of data, especially when folks upload files (using FileVars). So before I start hacking anything, I'd like to touch base if we agree on this path forward as it has quite some drawbacks.

The obvious remediation for this would be a scheduler which purges request data from the DB after a fairly low time frame (15m, 1h, 1d?), so we don't collect too much bloat.

Any thoughts @jeremystretch?

@jeremystretch jeremystretch removed the status: accepted This issue has been accepted for implementation label May 22, 2024
@jeremystretch jeremystretch added status: backlog Awaiting selection for work complexity: low Requires minimal effort to implement labels May 22, 2024
@8ctorres
Copy link

Hi! Just to chime in on this one. I do this exact thing for my NetBox users.

We have a few scripts that make changes to devices and circuits, and then the script returns the rendered configuration for that device, and the workflow the users follow is:

1st - Run the script without committing changes. The script makes the changes, returns the new rendered configuration, and doesn't save the changes.
2nd - Take the rendered configuration, go make whatever physical changes to the device (plug/unplug whatever cable) and then apply the new config
3rd - Check everything works as it should
4th - Run the same script again, with the same inputs, but this time committing the changes so they get saved to NetBox.

Step 4 is a bit tedious since it means the user has to go back and type in the same inputs again, and is also prone to human errors. What I do is, I generate a link to the script page that includes all of the attributes and their values, and print it in the output log of the script with the name "CLICK HERE TO RUN AGAIN" (it supports Markdown so it shows up as a hyperlink). When they click that, it takes them to the script input page but everything is already populated with the correct values, they only have to check the "commit" checkbox and hit "run".

The code is really simple:

    link = self.request.path + "?"
    for d in data:
        if type(data[d]) == str or type(data[d]) == int:
            # For basic types, make it url safe
            link = link + d + "=" + urllib.parse.quote_plus(str(data[d])) + "&"
        else:
            # For Netbox types, use object ID
            link = link + d + "=" + (str(data[d].id) + "&" 
    log_line = "[CLICK HERE TO RUN AGAIN](" + link[:-1] + ")"
    self.log_info(log_line)    

It's admittely a bit of a hack, and an actual button with this functionality would look better and also be less prone to errors, but it is a way to get around this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: low Requires minimal effort to implement status: backlog Awaiting selection for work type: feature Introduction of new functionality to the application
Projects
None yet
Development

No branches or pull requests

5 participants