Skip to content

Add request to Custom Script run, if receiver supports it #3775

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

Merged
merged 1 commit into from
Dec 26, 2019
Merged

Add request to Custom Script run, if receiver supports it #3775

merged 1 commit into from
Dec 26, 2019

Conversation

steffann
Copy link
Contributor

Fixes: #3705

  • Check whether the script.run() method accepts request as a keyword argument
  • If it does, pass the current request to the method

@steffann
Copy link
Contributor Author

steffann commented Dec 17, 2019

I implemented this as a keyword-only option to prevent any clashes with existing scripts that already have a run() method with multiple arguments. It's impossible to know how users have implemented their current scripts, better be safe than sorry…

@jeremystretch
Copy link
Member

Rather than altering the signature of run(), my plan was to make the current request available as a context in the Script object. For example, the script author would access self.request.user to get the current user (if any). This avoids the need to adapt existing scripts, and allows for cleanly running scripts without a request present (e.g. when executed locally via cron job).

@steffann
Copy link
Contributor Author

No problem, I considered both approaches but wasn't sure what the lifetime of a Script object was, and I didn't want to keep a long-lived reference to the request hanging around in a property. If that's not a problem then I agree your approach is cleaner.

I'll adapt the implementation!

@jeremystretch jeremystretch changed the base branch from develop-2.7 to develop December 19, 2019 19:09
@jeremystretch jeremystretch changed the base branch from develop to develop-2.7 December 19, 2019 19:10
@jeremystretch
Copy link
Member

@steffann Can you re-base this against the develop branch please? I don't think there's any reason to wait for v2.7 as it won't be a disruptive change.

@steffann steffann changed the base branch from develop-2.7 to develop December 19, 2019 22:36
@steffann
Copy link
Contributor Author

Done. In the end I just reset to develop and cherry-picked the patch.

@jeremystretch jeremystretch merged commit 84a2b72 into netbox-community:develop Dec 26, 2019
@steffann steffann deleted the 3705-make-current-user-available-in-custom-scripts branch January 23, 2020 14:42
@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make current user available in Custom Scripts
2 participants