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

python3Packages.netbox-floorplan-plugin: init at 0.6.0 #367107

Closed
wants to merge 3 commits into from

Conversation

Chaostheorie
Copy link
Contributor

Add the netbox-floorplan-plugin. This plugin can be enabled in netbox as netbox_floorplan. The used source variant is the official Netbox Community version. I've also added myself as a maintainer for this plugin.

Related to #261522, I will likely add PRs for more official plugins in the next few weeks.

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:
  • 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.

Add a 👍 reaction to pull requests you find important.

@Chaostheorie
Copy link
Contributor Author

cc maintainers of netbox for review: @minijackson / @RaitoBezarius / @n0emis

@felbinger
Copy link
Member

felbinger commented Dec 21, 2024

I will likely add PRs for more official plugins in the next few weeks.

Nice 🤩
If you need assistance mention me. Tried to package some of them myself, but haven't touched it for some time: https://github.com/secshellnet/nixos/tree/main/modules/netbox-plugins

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes labels Dec 21, 2024
@felbinger
Copy link
Member

I haven't used this plugin before but in my netbox instance it doesn't seam to be working as in the demo (see upstream README)... Non of these dialogs (see image) are working. Do they work for you?
image

@bzadm
Copy link

bzadm commented Jan 1, 2025

@felbinger this does work on my instance and was tested with #368036 and netbox from 24.11. My client is Firefox from 24.11.

If I remember correctly, you first have to upload a Floorplan image in the settings for the plug-in before you can use the UI you have shown. I will try to setup a new clean instance later for testing.

@felbinger
Copy link
Member

felbinger commented Jan 1, 2025

@felbinger this does work on my instance and was tested with #368036 and netbox from 24.11. My client is Firefox from 24.11.

If I remember correctly, you first have to upload a Floorplan image in the settings for the plug-in before you can use the UI you have shown. I will try to setup a new clean instance later for testing.

Weird: Same setup. I tried uploading a floorplan (which was successful), but it didn't work eighter. To be fair: I tested it in a netbox instance which has been running for a long time, so this might be a problem.

@felbinger
Copy link
Member

Don't forget to rebase the last three commits into one (git rebase -i HEAD~3, change last two to fixup and git push -f)

@felbinger
Copy link
Member

Have you been able to confirm that the plugin works in a fresh instance? Even with another browser it doesn't seam to be working on my instances.

@bzadm
Copy link

bzadm commented Jan 3, 2025

Sorry, I did not have time to test it at work yet. Will try to do it sometime next week.

@bzadm
Copy link

bzadm commented Jan 8, 2025

Tested on a fresh instance today (running 4.1.10 from #368036). The plugin is working fine.

It would be helpful if you could please check at the logs of your netbox instance/ your browser to check if an error has occurred somewhere. If an error has occurred, please share the logs and let's figure out if this is packaging related or an upstream issue.

@felbinger
Copy link
Member

Tested on a fresh instance today (running 4.1.10 from #368036). The plugin is working fine.

It would be helpful if you could please check at the logs of your netbox instance/ your browser to check if an error has occurred somewhere. If an error has occurred, please share the logs and let's figure out if this is packaging related or an upstream issue.

Figured it out. Had to run netbox-manage collectstatic, because the netbox instance had been started before. Sorry that I forgot to check before.

@Chaostheorie
Copy link
Contributor Author

@felbinger if I did the rebase correctly than the last three commits should now be squashed with a proper commit message. You were retained as a co-author.

@felbinger
Copy link
Member

@felbinger if I did the rebase correctly than the last three commits should now be squashed with a proper commit message. You were retained as a co-author.

Any reasons for not squashing the last three (init, fixup 1, fixup 2) into just the init commit? No need for the co-author, if you don't want to have it on your "init 0.5.0" commit.

@Chaostheorie
Copy link
Contributor Author

Any reasons for not squashing the last three (init, fixup 1, fixup 2) into just the init commit? No need for the co-author, if you don't want to have it on your "init 0.5.0" commit.

There was no particular reason. I've squashed the other two too now.

Regardless, I noticed that 0.6.0 was released upstream (for netbox 4.2.0 support). Should I update this PR for the new release of the plugin or open a second PR after this is merged?

@felbinger
Copy link
Member

I would update this pr.

@Chaostheorie Chaostheorie changed the title python3Packages.netbox-floorplan-plugin: init at 0.5.0 python3Packages.netbox-floorplan-plugin: init at 0.6.0 Jan 12, 2025
@Chaostheorie
Copy link
Contributor Author

PR was updated. I don't have a 4.2 instance to properly test the support of the plugin, will try later on a 4.1.10 instance.

@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 15, 2025
@Chaostheorie
Copy link
Contributor Author

Chaostheorie commented Jan 15, 2025

I will rebase this asap, apparently a merge conflicts also needs to be resoled.

Note: also tested this on a 4.1.10 instance, still worked fine for me.

@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Jan 15, 2025
@felbinger
Copy link
Member

felbinger commented Jan 16, 2025

Hi, when are you planning on rebasing it? With the "Merge branch master into master" this will not be mergeable...

If you want to I can assist you finishing it

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jan 17, 2025

We opened #374618 to remove the merge commit. I couldn't push to your fork because you opened the PR from master.

@Chaostheorie
Copy link
Contributor Author

Thank you for taking over the last rebase and making a new PR. I'll try to open new PRs based on the feedback for this one.

@felbinger
Copy link
Member

Thank you for taking over the last rebase and making a new PR. I'll try to open new PRs based on the feedback for this one.

👍 Feel free to contact me for collaboration, I packaged a bunch of plugins yesterday (dns, napalm, topology views, ...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: python 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants