Bake cookie from nautobot-plugin-v1.0 #100

snaselj · 2023-08-01T12:53:58Z

Closes NaN

What's Changed

Rebaked cookie from nautobot-plugin-v1.0

.gitignore

glennmatthews · 2023-08-01T13:20:10Z

development/development.env

+NAUTOBOT_DJANGO_EXTENSIONS_ENABLED=True
+NAUTOBOT_DJANGO_TOOLBAR_ENABLED=True


I'm not familiar with these two flags? I don't see any reference to them in core or in this PR.

From the template as well: https://github.com/networktocode-llc/cookiecutter-ntc/blob/nautobot-plugin-v1.0/nautobot-plugin/%7B%7B%20cookiecutter.project_slug%20%7D%7D/development/development.env#L10

Should I remove these 2 lines?

I think so?

@cmsirbu Seems like these env variables are not used in the nautobot-plugin template, should I remove it from there?

Go for it, opening a cleanup PR doesn't hurt... and might give you feedback from folks that remember why they were there.

It's still used in templates to enable Django debug toolbar.

https://github.com/networktocode-llc/cookiecutter-ntc/blob/527dedf686f6ea3cacad773cd7b7e96ee5c1f6ea/nautobot-plugin-ssot/%7B%7B%20cookiecutter.project_slug%20%7D%7D/development/nautobot_config.py#L343

Will align all templates to use the same envs and configuration for this. Keeping it here for now.

glennmatthews · 2023-08-01T13:23:00Z

development/docker-compose.postgres.yml

+      - "development.env"
+      - "creds.env"
+    volumes:
+      # - "./nautobot.sql:/tmp/nautobot.sql"


I would rather keep it aligned with the template: https://github.com/networktocode-llc/cookiecutter-ntc/blob/nautobot-plugin-v1.0/nautobot-plugin/%7B%7B%20cookiecutter.project_slug%20%7D%7D/development/docker-compose.postgres.yml#L14

Will probably be removed with the next template mint.

It's not gone in main either, might want to include it in the cleanup PR if it's not useful at all https://github.com/networktocode-llc/cookiecutter-ntc/blob/main/nautobot-plugin/%7B%7B%20cookiecutter.project_slug%20%7D%7D/development/docker-compose.postgres.yml

Its still used in nautobot template.

https://github.com/networktocode-llc/cookiecutter-ntc/blob/527dedf686f6ea3cacad773cd7b7e96ee5c1f6ea/nautobot/%7B%7B%20cookiecutter.project_slug%20%7D%7D/tasks.py#L258

Will replace these tasks in the template with the newer version, that doesn't require this binded volume. Keeping it here for now.

glennmatthews · 2023-08-01T13:23:30Z

development/nautobot_config.py

@@ -1,4 +1,5 @@
 """Nautobot development configuration file."""
+# pylint: disable=invalid-envvar-default


Not a big fan of global pylint disables. What specific issues is this hiding?

Agreed, but template contain it: https://github.com/networktocode-llc/cookiecutter-ntc/blob/nautobot-plugin-v1.0/nautobot-plugin/%7B%7B%20cookiecutter.project_slug%20%7D%7D/development/nautobot_config.py#L2

glennmatthews · 2023-08-01T13:24:09Z

development/nautobot_config.py

-DEBUG = is_truthy(os.getenv("NAUTOBOT_DEBUG", True))
+DEBUG = True


Why remove this configurability?

glennmatthews · 2023-08-01T13:25:35Z

docs/admin/compatibility_matrix.md

+!!! warning "Developer Note - Remove Me!"
+    Explain how the release models of the plugin and of Nautobot work together, how releases are supported, how features and older releases are deprecated etc.


What's the plan for addressing all these "Remove Me!" items and generally making sure all these newly added docs are actually relevant/accurate to the app? Part of this PR or future work?

Will fix these

@cmsirbu What do you think about the documentation? Does it make sense to rewrite it from README and if so, to do it in the separater PR?

Once an app has docs, I think we should not attempt to update markdown files. Stick to assets/css/requirements etc.

After the settlement with @cmsirbu , removed all changes to docs. Kept updated docs requirements in pyproject.toml and docs/requirements. Docs update should be done here: #54

glennmatthews · 2023-08-01T13:27:08Z

pyproject.toml

-# Pin this to the lowest supported version of Nautobot
-nautobot = "^1.4.0"


Why removed?

glennmatthews · 2023-08-01T13:27:44Z

pyproject.toml

+Markdown = "*"
+toml = "*"
+# Rendering docs to HTML
+mkdocs = "1.3.1"
+# Material for MkDocs theme
+mkdocs-material = "8.4.2"
+# Render custom markdown for version added/changed/remove notes
+mkdocs-version-annotations = "1.0.0"
+# Automatic documentation from sources, for MkDocs
+mkdocstrings = "0.19"
+mkdocstrings-python = "0.7.1"


Normally we've tried to keep dependencies in alphabetical order in pyproject.toml for ease of maintenance.

https://github.com/networktocode-llc/cookiecutter-ntc/blob/nautobot-plugin-v1.0/nautobot-plugin/%7B%7B%20cookiecutter.project_slug%20%7D%7D/pyproject.toml#L30C1-L30C1

Will sort it in the templates, keeping it here for now.

glennmatthews · 2023-08-01T13:29:14Z

tasks.py

-    command = (
-        "nautobot-server --config=nautobot_secrets_providers/tests/nautobot_config.py makemigrations --dry-run --check"
-    )
+    command = "nautobot-server --config=nautobot/core/tests/nautobot_config.py makemigrations --dry-run --check"


This looks wrong to me...

https://github.com/networktocode-llc/cookiecutter-ntc/blob/nautobot-plugin-v1.0/nautobot-plugin/%7B%7B%20cookiecutter.project_slug%20%7D%7D/pyproject.toml#L30C1-L30C1

Should I revert it?

Unless I'm overlooking something it doesn't look like CI exercises this invoke command. I suspect if you try it locally with this change it will fail - I think it needs to be reverted but would need to test it to be sure.

Reverted, will fix it in the templates.

glennmatthews · 2023-08-01T13:29:44Z

tasks.py

@@ -340,7 +398,6 @@ def check_migrations(context):
        "label": "specify a directory or module to test instead of running all Nautobot tests",
        "failfast": "fail as soon as a single test fails don't run the entire test suite",
        "buffer": "Discard output from passing tests",
-        "verbose": "Enable verbose test output.",


Why removed here when it's present at line 346/403 below as a kwarg?

snaselj · 2023-08-03T13:52:04Z

All comments are addressed now, ready for review.

bryanculver · 2023-08-09T11:48:03Z

nautobot_secrets_providers/__init__.py

-    base_url = "secrets"
+    base_url = "secrets-providers"


Breaking change?

bryanculver · 2023-08-09T11:48:46Z

invoke.example.yml

-  project_name: "nautobot_secrets_providers"
-  nautobot_ver: "1.4.10"
+  project_name: "nautobot-secrets-providers"
+  nautobot_ver: "latest"


This doesn't match what's in the tasks.py

Will fix the template, keeping it here for now.

https://github.com/networktocode-llc/cookiecutter-ntc/pull/368/commits/063a7ffc337c8dc552e5adbce53027203ea7f79d

bryanculver · 2023-08-09T11:50:43Z

pyproject.toml

-version = "1.4.1-beta.1"
+version = "1.4.1b1"


We follow the 1.4.1-beta.1 convention in Core. Is there a reason to change this?

I know there is a test failing but I think that test can be removed/changed.

We should IMHO use normalized version in pyproject.toml to have Docker image tags on par with pip package versions.

Interesting... Reading Poetry documentation, it's version command, PEP440 (https://peps.python.org/pep-0440/#semantic-versioning) it should have always been this way. Curious @nautobot/maintain-core, any prior knowledge why we use the full SemVer in Core?

Seems like this change/clarification happen last year: python-poetry/poetry#5576

bryanculver · 2023-08-09T11:52:19Z

.github/workflows/upstream_testing.yml

-      plugin_name: "nautobot-secrets-provider"
+      plugin_name: "nautobot-plugin-secrets-providers"


Want to make sure we're using the right name. I see in some places we have -plugin and in other places we don't.

plugin_name input is used to composite Docker image name inside called action. Seems this input argument should be repository name, so the value provided here is correct as well as the template.

Maybe we should fix the argument name to repository-name, to better reflect what's expected.

bryanculver · 2023-08-09T11:52:56Z

.github/workflows/ci.yml

-          python-version: "3.10"
+          python-version: "3.9"


Reason to go to an older Python version? Template?

Yup, it's in the template. Should I bump it to 3.10 / 3.11 in the template? Newer Python versions has better performance.

No need to bump. I think this is yet another good case for cookie version 1.2

HanlinMiao · 2023-08-22T18:10:01Z

development/creds.example.env

+# Postgres
+POSTGRES_PASSWORD=${NAUTOBOT_DB_PASSWORD}
+PGPASSWORD=${NAUTOBOT_DB_PASSWORD}

-# Needed for Redis, must match the values for Nautobot above
-REDIS_PASSWORD=notverysecurepwd
+# MySQL Credentials
+MYSQL_ROOT_PASSWORD=${NAUTOBOT_DB_PASSWORD}
+MYSQL_PASSWORD=${NAUTOBOT_DB_PASSWORD}


Should we make a distinction between Postgres and MySQL credentials? It is rare that they use the same environmental variable as passwords right?

Personally I would prefer to remove binding from Docker Compose configs to these creds.env and development.env, and rather specify environment variables for each compose service independently. (Remove key env_file and use environment instead). The current solution exposes all environment files to most services, even when they do not consume it. However such change is not necessary now IMHO.

I use proposed solution here:

https://github.com/nautobot/cookiecutter-nautobot-app-drift-manager/blob/develop/development/compose.yaml

Environment is loaded and altered when using invoke.

https://github.com/nautobot/cookiecutter-nautobot-app-drift-manager/blob/develop/tasks.py#L38
https://github.com/nautobot/cookiecutter-nautobot-app-drift-manager/blob/develop/development/default.env#L1

bryanculver · 2023-08-23T12:51:51Z

.github/workflows/rebake.yml

+on:  # yamllint disable
+  schedule:
+    # every Saturday at 4:00
+    - cron: "0 4 * * 6"


Since this is open source and we want to make a lot of changes/test this out, let's add a manual trigger as well as run it every other night.

* chore: Drop Python 3.7 support * chore: Remove unnecessary importlib-metadata

bryanculver

If CI passes now, let's ship

- [Based on](nautobot/nautobot-app-secrets-providers#100). - Fixed invoke tasks: - Don't specify nautobot config for `invoke check-migrations` - Arguments added to invoke tasks: - `invoke unittest --verbose` - `invoke tests --keepdb` - `invoke tests --lint-only` - Removed obsolete environment variables: - `NAUTOBOT_DJANGO_EXTENSIONS_ENABLED` - `NAUTOBOT_DJANGO_TOOLBAR_ENABLED` - Better organized `development/nautobot_config.py` file. - Removed unnecessary linter rules. - Moved `Debug` section to the top, to be easily reused. - Moved `Redis` section next to `Database` section. - Renamed `TESTING` to `_TESTING` as it's the local variable only. - Removed `RQ` related comments. - Renamed `plugin` => `App` in comments. - Removed obsoleted commented out docker-compose postgres mount `nautobot.sql`. - Alphabetically sorted requirements and removed obvious comments. - Aligned nautobot-plugin and nautobot-plugin-ssot templates: - `development/nautobot_config.py` - invoke tasks - docs requirements and configuration fix: Tasks formatting fix: Use `plugin_slug` for compose project name

* Move poetry dev dependencies to tool.poetry.group.dev.dependencies * Update .flake8 * feat: Support Drift Manager fix: yamllint * fix: Comments from secrets providers PR - [Based on](nautobot/nautobot-app-secrets-providers#100). - Fixed invoke tasks: - Don't specify nautobot config for `invoke check-migrations` - Arguments added to invoke tasks: - `invoke unittest --verbose` - `invoke tests --keepdb` - `invoke tests --lint-only` - Removed obsolete environment variables: - `NAUTOBOT_DJANGO_EXTENSIONS_ENABLED` - `NAUTOBOT_DJANGO_TOOLBAR_ENABLED` - Better organized `development/nautobot_config.py` file. - Removed unnecessary linter rules. - Moved `Debug` section to the top, to be easily reused. - Moved `Redis` section next to `Database` section. - Renamed `TESTING` to `_TESTING` as it's the local variable only. - Removed `RQ` related comments. - Renamed `plugin` => `App` in comments. - Removed obsoleted commented out docker-compose postgres mount `nautobot.sql`. - Alphabetically sorted requirements and removed obvious comments. - Aligned nautobot-plugin and nautobot-plugin-ssot templates: - `development/nautobot_config.py` - invoke tasks - docs requirements and configuration fix: Tasks formatting fix: Use `plugin_slug` for compose project name * Min bump to Python 3.8 and Core 1.6 Update nautobot-plugin/{{ cookiecutter.project_slug }}/pyproject.toml Co-authored-by: Christian Adell <chadell@gmail.com> Address review feedback Update docs links Update link Copy changes to ssot template * Merge misses * Delete nautobot-app/{{ cookiecutter.project_slug }}/.github/workflows/rebake.yml * Address feedback * Save before commit * Alpha sorted features only * Duplicate docs deps --------- Co-authored-by: itdependsnetworks <ken@celenza.org> Co-authored-by: nniehoff <nick.niehoff@networktocode.com> Co-authored-by: Jan Snasel <jan.snasel@networktocode.com> Co-authored-by: Cristian Sirbu <cmsirbu@users.noreply.github.com>

* chore: First rebake without docs * docs: From first rebake * fix: Dependencies * fix: Format * fix: Install all deps * fix: Normalize package version * fix: pylint issue * fix: PR comments * fix: invoke check-migrations * revert: Docs * fix: Removed obsoleted docker-compose.docs.yml * fix: Revert base_url change * feat: Auto rebake cookie * fix: Run every day and add a manual trigger * Drop Python 3.7 support (nautobot#102) * chore: Drop Python 3.7 support * chore: Remove unnecessary importlib-metadata

snaselj added 6 commits August 1, 2023 12:38

chore: First rebake without docs

5b80cea

docs: From first rebake

0f5b7b9

fix: Dependencies

e29d379

fix: Format

8867d6e

fix: Install all deps

322fb29

fix: Normalize package version

4c2c7ab

glennmatthews reviewed Aug 1, 2023

View reviewed changes

snaselj added 5 commits August 1, 2023 14:23

fix: pylint issue

e488c45

fix: PR comments

d733b50

fix: invoke check-migrations

0405a37

revert: Docs

16dd78b

fix: Removed obsoleted docker-compose.docs.yml

b5bd297

snaselj marked this pull request as ready for review August 3, 2023 13:51

snaselj requested a review from a team as a code owner August 3, 2023 13:51

bryanculver reviewed Aug 9, 2023

View reviewed changes

fix: Revert base_url change

bcdf584

bryanculver requested a review from glennmatthews August 21, 2023 13:00

bryanculver added emergent Unplanned work that is brought into a sprint after it's started. type: housekeeping labels Aug 21, 2023

bryanculver assigned timizuoebideri1 Aug 22, 2023

bryanculver approved these changes Aug 22, 2023

View reviewed changes

jathanism approved these changes Aug 22, 2023

View reviewed changes

HanlinMiao approved these changes Aug 22, 2023

View reviewed changes

snaselj mentioned this pull request Aug 23, 2023

Rename plugin_name input argument to repository-name nautobot/nautobot#4306

Open

feat: Auto rebake cookie

2ee490b

bryanculver requested changes Aug 23, 2023

View reviewed changes

snaselj added 2 commits August 23, 2023 13:09

fix: Run every day and add a manual trigger

359f4a8

Drop Python 3.7 support (#102)

f3872d8

* chore: Drop Python 3.7 support * chore: Remove unnecessary importlib-metadata

bryanculver approved these changes Aug 24, 2023

View reviewed changes

bryanculver merged commit 5acedaf into develop Aug 24, 2023
15 checks passed

snaselj deleted the drift-manager/pr branch August 25, 2023 11:16

		NAUTOBOT_DJANGO_EXTENSIONS_ENABLED=True
		NAUTOBOT_DJANGO_TOOLBAR_ENABLED=True

		@@ -1,4 +1,5 @@
		"""Nautobot development configuration file."""
		# pylint: disable=invalid-envvar-default

		DEBUG = is_truthy(os.getenv("NAUTOBOT_DEBUG", True))
		DEBUG = True

		!!! warning "Developer Note - Remove Me!"
		Explain how the release models of the plugin and of Nautobot work together, how releases are supported, how features and older releases are deprecated etc.

		# Pin this to the lowest supported version of Nautobot
		nautobot = "^1.4.0"

		plugin_name: "nautobot-secrets-provider"
		plugin_name: "nautobot-plugin-secrets-providers"

Bake cookie from nautobot-plugin-v1.0 #100

Bake cookie from nautobot-plugin-v1.0 #100

Conversation

snaselj commented Aug 1, 2023

Closes NaN

What's Changed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snaselj Aug 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

snaselj commented Aug 3, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryanculver left a comment

Choose a reason for hiding this comment

snaselj Aug 1, 2023 •

edited

Loading