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

netbox: 4.1.3 -> 4.1.10 #368036

Closed
wants to merge 12 commits into from
Closed

netbox: 4.1.3 -> 4.1.10 #368036

wants to merge 12 commits into from

Conversation

bzadm
Copy link

@bzadm bzadm commented Dec 25, 2024

Update to latest 4.1.10 version, it should be noted that a testing version for 4.2 is already available.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests (.#netbox.passthru.tests.netbox
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Supersedes #366765


Add a 👍 reaction to pull requests you find important.

Update to latest 4.1.x version, it should be noted that a testing
version for 4.2 is already available.
@bzadm
Copy link
Author

bzadm commented Dec 25, 2024

On an unrelated note, I noticed that django-rich was introduced as a dependency in netbox 4 but isn't packaged in nixpkgs nor contained in the dependencies for the netbox derivation. It obviously works without it, but why was it skipped?

@bzadm
Copy link
Author

bzadm commented Dec 25, 2024

Reverting to draft, 4.10 requires a thorough update of dependencies. At least django-rq (due to rq) had major version bump with breaking changes.

@bzadm
Copy link
Author

bzadm commented Dec 25, 2024

An overview of where nixpkgs is and what netbox requires (satisfied-ish versions were skipped):

Dependency netbox requirement nixpkgs master note
django-rq 3.0 2.10.1 This is due to rq v2, which is breaking.
django-cors-headers 4.6.0 4.4.0 Not breaking but out of date
django-htmx 1.19.0 1.21.0 likely not important
django-taggit 6.1.0 5.0.1
drf-spectacular 0.28.0 0.27.2 This adds some django 5.1 support based on the release notes
drf-spectacular-sidecar 2024.12.1 2023.9.1
nh3 0.2.20 0.2.17 only on develop, 4.1.10.
strawberry-graphql 0.254.0 0.253.1 only on develop, not 4.1.10
strawberry-django 0.52.0 0.47.1 only on develop, not 4.1.10
tablib 3.7.0 3.6.1

edit: updates for requirements.txt on tag v4.1.10

bzadm added 10 commits December 25, 2024 04:08
Create a separate package for the new major version (with breaking
changes) of rq. This is an intermediate fix to address netbox however
updating python3Packages.rq will likely also be a good idea soon-ish.
For new major version of rq, see previous commit in 368036.
This is an intermediate fix for netbox and may be removed in favor of
updating django-rq directly.
This vendors and updates the patch for django 5.1 support in netbox
4.1.x. This is required because nixpkgs used django 5.1 but 4.1.x uses
django 4.
@bzadm
Copy link
Author

bzadm commented Dec 25, 2024

The patch for Django 5.1 also requires an adjustment for the new requirements.txt, fun. I've included all dependency updates in this PR, this can be split into multiple PRs if required.

They don't work. I don't fully understand why but the tests, nor
strawberry-graphql, are unstable so this likely is just a development
artifact.
@bzadm
Copy link
Author

bzadm commented Dec 25, 2024

cc maintainers of affected packages: @Izorkin @minijackson @RaitoBezarius @mweinelt @happysalada

@happysalada
Copy link
Contributor

Lgtm

@minijackson
Copy link
Member

Thanks a lot! There are quite a few packages that fail during nixpkgs-review (which I didn't manage to finish, my laptop run out of memory).

I'm also not quite sure if we should be doing separate rq2 and django-rq3 packages, this method is usually reserved if all versions are supported upstream. This could lead to other packges using outdated / unsupported dependencies versions. Tagging @mweinelt for guidance and as maintainer of the django-rq packages and other Python packages.

I can confirm that NetBox tests passes, though!

@bzadm
Copy link
Author

bzadm commented Dec 25, 2024

@minijackson thank you for trying to get nixpkgs-review running. My PC did not finish it either. It appears this touches at least a few hundred packages, likely due to rq etc., and will be quite hefty rebuild.

For at least netbox I can now also confirm that this works with plug-ins (except netbox-documents, checks appear broken) in a full deployment. I updated an existing 4.1.3 instance to 4.1.10 without issues.

For rq2/django-rq3 my main concern is that other upstream netbox versions like 4.0.x and 3.x relay on rq 1.x. They are supposedly breaking API changes and at least netbox 4.0.x relies on rq 1.x (though maybe it can work with v2, idk).

@wegank wegank added 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Dec 26, 2024
@wegank wegank added 2.status: merge conflict This PR has merge conflicts with the target branch and removed 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in the package labels Jan 3, 2025
@bzadm
Copy link
Author

bzadm commented Jan 8, 2025

netbox has released 4.2.0 and 4.2.1 today. Should I close this PR in favor of updating to 4.2.x directly?

@minijackson
Copy link
Member

I'd normally say that packaging 4.1.10 is still useful to backport to 24.11, but it seems even 4.1.10 might prove difficult to backport…

@bzadm
Copy link
Author

bzadm commented Jan 8, 2025

Okay, then I will close this PR for now and refactore it for 4.2.1. As far as I can tell this should mostly be just a version bump (for packaging) and some more testing.

I will also spin off the approved changes for the python dependencies into their own MRs.

@bzadm bzadm closed this Jan 8, 2025
@bzadm
Copy link
Author

bzadm commented Jan 23, 2025

Superseded by #376100

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: python 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 101-500 10.rebuild-linux: 101-500 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants