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

Remove support for reports in NetBox v4.0 #12510

Closed
jeremystretch opened this issue May 5, 2023 · 7 comments · Fixed by #14976
Closed

Remove support for reports in NetBox v4.0 #12510

jeremystretch opened this issue May 5, 2023 · 7 comments · Fixed by #14976
Assignees
Labels
breaking change This change modifies or removes some previously documented functionality status: accepted This issue has been accepted for implementation type: deprecation Removal of existing functionality or behavior
Milestone

Comments

@jeremystretch
Copy link
Member

Proposed Changes

Remove all support for custom reports. Support for custom scripts will remain in place.

This change is being proposed for NetBox v4.0 to allow users ample time to prepare for the migration.

Justification

There's a lot of overlap in functionality between reports and scripts, and dropping reports in favor of scripts will allow us to simplify the user experience and remove a substantial amount of largely redundant code.

Impact

All reports will need to be converted to scripts, which should be fairly straightforward. We can produce supporting documentation to assist users with this process.

@jeremystretch jeremystretch added status: under review Further discussion is needed to determine this issue's scope and/or implementation type: deprecation Removal of existing functionality or behavior labels May 5, 2023
@sleepinggenius2
Copy link
Contributor

We primarily use reports for compliance and referential integrity between multiple systems today. From a UX perspective, having the Run Again button, the individual test cases with their status badges, and the ability to reference an object directly within the logs are critical to our workflow. I'm all for consolidating these two pieces of functionality, as it would help with a lot of duplicate code on our end too, but it's not clear to me what the straightforward path to convert from a report to a script is at this point. I know Nautobot did something like this with their Jobs functionality by incorporating run(), test_*(), and post_run() methods within the same class. My vote would be to merge the existing Script and Report functionality in a similar way. It looks like they've added some other nice features, like job buttons within object views and approval functionality as well.

@jeremystretch jeremystretch added this to the v4.0 milestone Jun 15, 2023
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Jun 15, 2023
@ITJamie
Copy link
Contributor

ITJamie commented Aug 16, 2023

from a permissions POV we have reports and scripts separate right now. once this happens any user who needs to run a report will need the run script permission. this will allow users a higher level of access to make changes

Having a specific permission for scripts that would set whether the user can run scripts which make changes or not would be good.
otherwise there's a risk that a user could run a script thinking it just was a "report" and it making changes.

the current split between reports + scripts effectively handles this but once combined it changes the risk profile

@ross-cello
Copy link

Heya team, regarding this report deprecation and the mention of supporting documentation and/or transitional function for Scripts; do you happen to have a target minor release prior to 4.0 for these considerations?

I'm also mindful of permissions in particular, but appreciate that none of these aspects might be on your radar yet.

@jeremystretch
Copy link
Member Author

@ITJamie raises a good point about a core difference between reports and scripts. Reports are intended (but are not technically required) to be read-only, whereas scripts generally make changes to data. The other core difference IMO is that reports serve to produce well-formatted pass/fail results, whereas scripts are completely open-ended.

Maybe instead of trying to mash these two resources into one, we'd be better served by further differentiating them. One improvement could be to restrict database write operations during the execution of a report, ensuring that a report cannot inadvertently alter data.

@ITJamie
Copy link
Contributor

ITJamie commented Dec 7, 2023

If we do extend reports instead of depreciating it, we should extend reports to display returned data like scripts can.

We have a few "reports" that we have to run as scripts so they can return csv or json data.

The actual report/script classes under the hood could be merged or have a shared parent class in the longterm to get the featureset and response methods to be the similar but still easily maintainable

@DanSheps
Copy link
Member

Maybe instead of trying to mash these two resources into one, we'd be better served by further differentiating them. One improvement could be to restrict database write operations during the execution of a report, ensuring that a report cannot inadvertently alter data.

Could we maybe do a hybrid approach?

  • Merge reports in with scripts
  • A "script" is a report if it inherits from the "Reports" Superclass
  • Reports a restricted from modifying non-report objects

This might also open up the ability to write scripts that can display that nicely formatted pass/fail result that reports can. It also allows overall efficiencies under the hood.

@jeremystretch jeremystretch removed this from the v4.0 milestone Dec 29, 2023
@jeremystretch jeremystretch added status: under review Further discussion is needed to determine this issue's scope and/or implementation and removed status: accepted This issue has been accepted for implementation labels Dec 29, 2023
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels Jan 11, 2024
@jeremystretch jeremystretch added this to the v4.0 milestone Jan 11, 2024
@jeremystretch
Copy link
Member Author

We decided in the maintainers' meeting today to proceed with this change, merging reports into scripts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking change This change modifies or removes some previously documented functionality status: accepted This issue has been accepted for implementation type: deprecation Removal of existing functionality or behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants