Skip to content

Check for file in ENABLED_PLUGINS#2045

Merged
mdellweg merged 1 commit into
pulp:mainfrom
mdellweg:depend_on_file
Sep 4, 2025
Merged

Check for file in ENABLED_PLUGINS#2045
mdellweg merged 1 commit into
pulp:mainfrom
mdellweg:depend_on_file

Conversation

@mdellweg
Copy link
Copy Markdown
Member

No description provided.

Comment thread pulp_container/app/checks.py Outdated

# pulp_container depends on pulp_file so if ENABLED_PLUGINS is used, it must be contained.
if ENABLED_PLUGINS := getattr(settings, "ENABLED_PLUGINS", None) is not None:
if "file" not in ENABLED_PLUGINS:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's pulp_file, no?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes it is.

Comment thread pulp_container/app/checks.py Outdated
@mdellweg
Copy link
Copy Markdown
Member Author

That approach didn't work for me...

@mdellweg mdellweg marked this pull request as draft August 13, 2025 15:16
@mdellweg
Copy link
Copy Markdown
Member Author

And without buyin on the pulpcore side, i don't see that we can add validation for the settings.
@pedro-psb any idea on the dynaconf side of things?

@pedro-psb
Copy link
Copy Markdown
Member

Sorry, somehow I didn't see that before.

So yeah, this should be validated in the settings, because the error is raised before those django checks.

The easiest way is adding a dynaconf hook.
It could even append "pulp_file" there if its absence, if that's not too much of a surprise.

--- /dev/null
+++ b/pulp_container/app/dynaconf_hooks.py
@@ -0,0 +1,10 @@
+# dynaconf_hooks.py
+from dynaconf import ValidationError
+
+
+def post(settings):
+    data = {"dynaconf_merge": True}
+    enabled_plugins = settings.get("ENABLED_PLUGINS")
+    if enabled_plugins and "pulp_file" not in enabled_plugins:
+        raise ValidationError("pulp_file must be enabled to use pulp_container")
+    return data

Or we could define some plugin config so plugins can declare which plugins they require, and let pulpcore validate it.

fixes pulp#2044

Co-authored-by: Evgeni Golov <evgeni@golov.de>
Co-authored-by: Pedro Brochado <pedropsb95@gmail.com>
@mdellweg mdellweg marked this pull request as ready for review September 4, 2025 08:03
@mdellweg
Copy link
Copy Markdown
Member Author

mdellweg commented Sep 4, 2025

I believe this works now. Thanks @pedro-psb

@mdellweg mdellweg merged commit d42faf1 into pulp:main Sep 4, 2025
22 of 25 checks passed
@mdellweg mdellweg deleted the depend_on_file branch September 4, 2025 11:47
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.

3 participants