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

Added devcontainer option #11

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jahknem
Copy link

@jahknem jahknem commented Jan 19, 2023

As proposed in issue #3 I've added a devcontainer option which adds a .devcontainer/ folder to the newly created project. This container includes a working netbox installation with the DEBUG and DEVELOPER flags set to True in the configuration.
Due to cookiecutter restrictions the .devcontainer/ folder is alway created and deleted afterwards if the devcontainer variable is not set to "True".

Initial tests look good. >95% of the code are from the Netbox-ACL Plugin

@arthanson
Copy link
Contributor

@jahknem thanks for adding this. Is all the initializer stuff needed? It adds a lot of bloat and I'm not sure everyone would want that in their repository, also it will make it potentially more work if NetBox changes and happens to change the models.

I'd rather keep it to just the devcontainer with the default empty DB and remove all the initialization. Also, same question on the custom .bashrc and .zshrc files. I'd like to keep the config pretty bare-bones and let developers add in extra stuff they need, otherwise it gets more complicated to understand, document and complex for the developer who has to rip stuff out. Basically add the minimum to get the developer started, but not extra stuff they may have to rip out.

@jahknem
Copy link
Author

jahknem commented Jan 23, 2023

@arthanson you are correct, that all is not needed. I will remove the bloat and update the fork shortly. I would however like to keep the venv activation, as I cannot see a use case where it is not needed in the container

EDIT: I cannot get it to work at the moment. Will take a look at it again tomorrow.

@arthanson
Copy link
Contributor

@jahknem any progress on this? Looks like a nice addition.

@jahknem
Copy link
Author

jahknem commented May 2, 2023

@arthanson sorry no. I've got problems with devcontainers in general, which I can't seem to pinpoint. I believe the issues mostly arise if a container already exists, but not yet sure. If you are willing to have a go at it, please do.
(Currently I also don't have the time as I just entered a new university for my masters)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants