Switches plugin reporting to use Django name #1102

bmbouter · 2021-02-01T21:35:41Z

The Status API and the OpenAPI schema API now both use the Django name.
This is a backwards incompatible change, but necessary to fix the bug
which was using Python package names incorrectly instead.

This also reapplies the plugin template changes required from the PR
below:

https://github.com/pulp/plugin_template/pull/341/files

closes #8198

bmbouter · 2021-02-01T21:40:24Z

This changes from

    "versions": [
        {
            "component": "pulpcore",
            "version": "3.10.0.dev0"
        },
        {
            "component": "pulp_ansible",
            "version": "0.7.0.dev0"
        },
        {
            "component": "pulp_file",
            "version": "1.6.0.dev0"
        }
    ]

To

    "versions": [
        {
            "component": "core",
            "version": "3.10.0.dev"
        },
        {
            "component": "ansible",
            "version": "0.7.0.dev"
        },
        {
            "component": "file",
            "version": "1.6.0.dev"
        }
    ],

mdellweg

Sounds good to me.
Just a coding style suggestion.

mdellweg · 2021-02-02T10:07:36Z

pulpcore/app/apps.py

                "pulpcore==3.10, plugins are required to define their version on the "
                "PulpPluginAppConfig subclass."
            )
-            warnings.warn(msg, FutureWarning)
+            raise ImproperlyConfigured(msg.format(self.label))


mdellweg · 2021-02-02T10:11:36Z

pulpcore/openapi/__init__.py

+        result["info"]["x-pulp-app-versions"] = {}
+        for app in pulp_plugin_configs():
+            result["info"]["x-pulp-app-versions"][app.label] = app.version


Suggested change

result["info"]["x-pulp-app-versions"] = {}

for app in pulp_plugin_configs():

result["info"]["x-pulp-app-versions"][app.label] = app.version

result["info"]["x-pulp-app-versions"] = {

app.label: app.version

for app in pulp_plugin_configs()

}

I'd like to decline this change if that's alright. The list comprehension with the iterator at the end is less readable for me. What I wrote is a bit slower though.

goosemania

Thanks! Looks good to me.

goosemania · 2021-02-02T11:15:49Z

This changes from

    "versions": [
        {
            "component": "pulpcore",
            "version": "3.10.0.dev0"
        },
        {
            "component": "pulp_ansible",
            "version": "0.7.0.dev0"
        },
        {
            "component": "pulp_file",
            "version": "1.6.0.dev0"
        }
    ]

To

    "versions": [
        {
            "component": "core",
            "version": "3.10.0.dev"
        },
        {
            "component": "ansible",
            "version": "0.7.0.dev"
        },
        {
            "component": "file",
            "version": "1.6.0.dev"
        }
    ],

@jlsherrill , FYI, this change is coming in 3.10. I think Katello uses the status endpoint to check pulp services state but I wonder if you check the installed plugins as well.

mdellweg · 2021-02-02T12:46:17Z

pulpcore/app/views/status.py

-        ]
+        versions = []
+        for app in pulp_plugin_configs():
+            versions.append({"component": app.label, "version": app.version})


I just realized, this changes the keys and not only the trailing "0" in the versions.
This will break the version check logic in the cli.
Can we get back to the old names`

I also thought about this, but I don't see an easy way to. :/ What this change does is hand control over to the plugin for the name it provides. So it's up to the plugins really. Let's talk about it at the pulpcore meeting, I put an agenda item on there.

And BTW. This is what breaks the CI here too.

bmbouter · 2021-02-02T18:40:57Z

This PR will become for 3.11. For 3.10 I've moved the non-breaking changes to this PR #1106

bmbouter · 2021-02-02T19:45:05Z

@mdellweg any link on where I should look to fix the CI? Also is this something I'll need to also fix in the plugin template?

mdellweg · 2021-02-02T20:11:26Z

@mdellweg any link on where I should look to fix the CI? Also is this something I'll need to also fix in the plugin template?

https://github.com/pulp/pulpcore/blob/master/.ci/ansible/start_container.yaml#L87 is the check that fails.
https://github.com/pulp/pulpcore/blob/master/.github/workflows/scripts/before_install.sh#L37 is where that variable is set.

mdellweg · 2021-02-02T20:14:08Z

Thinking more about this: The check is really valuable, as it catches oops-we-updated-the-thing-to-test-from-pypi-because-of-dependencies mistakes. But it relies on the assumption that there is only one plugin in the repository.

bmbouter · 2021-02-03T19:11:14Z

@mdellweg I agree it's valuable but we no longer have info required to perform it (right?). All I can think to do is to disable the check. What are our other options? Can you (or someone on the CI team) give me some advice? Thanks!

mdellweg · 2021-02-08T15:26:37Z

@mdellweg I agree it's valuable but we no longer have info required to perform it (right?). All I can think to do is to disable the check. What are our other options? Can you (or someone on the CI team) give me some advice? Thanks!

For our well curated "single_plugins_in_one_repo" we should still able to find the information what version it is supposed to be, right? I really want to avoid testing a released version instead of failing for dependency issues.

bmbouter · 2021-02-08T15:30:35Z

@mdellweg my apologies. I'm haven't looked into that CI check yet and I'm not up to speed on what it does. Can you elaborate a bit more? Thanks!

mdellweg · 2021-02-08T16:01:07Z

Sure, it makes sure, that the code that is installed inside the container is the one we are supposed to test.
We had previous incidents, where a plugin by not being compatible with the provided version of pulpcore downgraded pulpcore to a compatible one from pypi, and if it weren't for mismatching functional tests, this would have gone unnoticed.

mdellweg · 2021-02-17T16:48:55Z

.github/workflows/scripts/before_install.sh

@@ -34,7 +34,7 @@ else
 fi
 mkdir .ci/ansible/vars || true
 echo "---" > .ci/ansible/vars/main.yaml
-echo "component_name: pulpcore" >> .ci/ansible/vars/main.yaml
+echo "component_name: pulp" >> .ci/ansible/vars/main.yaml


Suggested change

echo "component_name: pulp" >> .ci/ansible/vars/main.yaml

echo "component_name: core" >> .ci/ansible/vars/main.yaml

mdellweg · 2021-02-17T16:49:13Z

template_config.yml

@@ -19,7 +19,7 @@ deploy_daily_client_to_rubygems: true
 deploy_to_pypi: true
 docker_fixtures: true
 docs_test: true
-plugin_app_label: pulpcore
+plugin_app_label: pulp


Suggested change

plugin_app_label: pulp

plugin_app_label: core

mdellweg · 2021-02-17T17:09:17Z

I'm about to say ACK here, but we need to merge
pulp/plugin_template#341
first and roll it out to all plugins (and their still tested stable branches) first.

bmbouter · 2021-02-17T18:49:00Z

@mdellweg I reapplied the plugin template from https://github.com/pulp/plugin_template/pull/341/files and pushed it here to see if this change will pass with that plugin template change.

The Status API and the OpenAPI schema API now both use the Django name. This is a backwards incompatible change, but necessary to fix the bug which was using Python package names incorrectly instead. This also reapplies the plugin template changes required from the PR below: https://github.com/pulp/plugin_template/pull/341/files closes #8198

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

pulpbot · 2021-02-23T15:35:09Z

Attached issue: https://pulp.plan.io/issues/8198

bmbouter force-pushed the make-status-api-use-versions-from-plugins branch 2 times, most recently from 7d55deb to 196be42 Compare February 1, 2021 21:38

mdellweg reviewed Feb 2, 2021

View reviewed changes

goosemania approved these changes Feb 2, 2021

View reviewed changes

mdellweg requested changes Feb 2, 2021

View reviewed changes

bmbouter marked this pull request as draft February 2, 2021 19:24

bmbouter force-pushed the make-status-api-use-versions-from-plugins branch 2 times, most recently from e1494d5 to 59e3887 Compare February 2, 2021 19:37

bmbouter force-pushed the make-status-api-use-versions-from-plugins branch from 59e3887 to 638262f Compare February 17, 2021 16:38

mdellweg reviewed Feb 17, 2021

View reviewed changes

bmbouter force-pushed the make-status-api-use-versions-from-plugins branch from 638262f to 312a7b4 Compare February 17, 2021 17:01

bmbouter force-pushed the make-status-api-use-versions-from-plugins branch from 312a7b4 to ce4a8a6 Compare February 17, 2021 18:48

bmbouter mentioned this pull request Feb 17, 2021

Change the version check to use app label pulp/plugin_template#341

Merged

bmbouter force-pushed the make-status-api-use-versions-from-plugins branch from ce4a8a6 to 721c8d1 Compare February 18, 2021 12:10

mdellweg added a commit to mdellweg/pulp_file that referenced this pull request Feb 18, 2021

Update plugin template

db43e4c

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg added a commit to mdellweg/pulp-2to3-migration that referenced this pull request Feb 18, 2021

Update plugin template

78d929f

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg added a commit to mdellweg/pulp_deb that referenced this pull request Feb 18, 2021

Update plugin template

1fad345

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg added a commit to mdellweg/pulp_deb that referenced this pull request Feb 18, 2021

Update plugin template

87327cd

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg mentioned this pull request Feb 18, 2021

Update plugin template pulp/pulp_deb#247

Closed

mdellweg added a commit to mdellweg/pulp_maven that referenced this pull request Feb 18, 2021

Update plugin template

dbeb57c

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg mentioned this pull request Feb 18, 2021

Update plugin template pulp/pulp_maven#36

Merged

mdellweg added a commit to mdellweg/pulp_npm that referenced this pull request Feb 18, 2021

Update plugin template

c0b2176

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg mentioned this pull request Feb 18, 2021

Update plugin template pulp/pulp_npm#70

Merged

mdellweg added a commit to mdellweg/pulp_python that referenced this pull request Feb 18, 2021

Update plugin template

0c5980c

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg mentioned this pull request Feb 18, 2021

Update plugin template pulp/pulp_python#330

Merged

mdellweg added a commit to mdellweg/pulp_rpm that referenced this pull request Feb 18, 2021

Update plugin template

8f5db40

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg mentioned this pull request Feb 18, 2021

Update plugin template pulp/pulp_rpm#1940

Merged

mdellweg added a commit to pulp/pulp_container that referenced this pull request Feb 19, 2021

Update plugin template

c6b8835

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg mentioned this pull request Feb 20, 2021

The version checks need to understand app_labels of plugins and repository names as a fallback pulp/pulp-cli#153

Closed

mdellweg added a commit to mdellweg/pulp-2to3-migration that referenced this pull request Feb 22, 2021

Update plugin template

5f88cbf

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg added a commit to mdellweg/pulp_rpm that referenced this pull request Feb 22, 2021

Update plugin template

b732de9

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

goosemania pushed a commit to pulp/pulp-2to3-migration that referenced this pull request Feb 22, 2021

Update plugin template

3b979d8

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

mdellweg added a commit to mdellweg/pulp_rpm that referenced this pull request Feb 22, 2021

Update plugin template

4f2a86e

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

ggainey pushed a commit to pulp/pulp-certguard that referenced this pull request Feb 22, 2021

Update plugin template

17dec2d

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

dralley pushed a commit to pulp/pulp_file that referenced this pull request Feb 22, 2021

Update plugin template

7e29f69

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

dralley pushed a commit to pulp/pulp_rpm that referenced this pull request Feb 22, 2021

Update plugin template

a03488c

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

bmbouter pushed a commit to pulp/pulp_maven that referenced this pull request Feb 22, 2021

Update plugin template

46bf60e

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

bmbouter pushed a commit to pulp/pulp_npm that referenced this pull request Feb 22, 2021

Update plugin template

1c4e7df

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

bmbouter pushed a commit to pulp/pulp_ansible that referenced this pull request Feb 22, 2021

Update plugin template

04df5e5

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

dralley pushed a commit to dralley/pulp_file that referenced this pull request Feb 22, 2021

Update plugin template

05ff437

This accommodates for a change in pulpcore that will effect the CI. pulp/pulpcore#1102 [noissue]

pulp deleted a comment from pulpbot Feb 23, 2021

mdellweg marked this pull request as ready for review February 25, 2021 15:03

mdellweg approved these changes Feb 25, 2021

View reviewed changes

bmbouter merged commit b027af8 into pulp:master Feb 25, 2021

bmbouter deleted the make-status-api-use-versions-from-plugins branch February 25, 2021 15:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switches plugin reporting to use Django name #1102

Switches plugin reporting to use Django name #1102

bmbouter commented Feb 1, 2021 •

edited

Loading

bmbouter commented Feb 1, 2021

mdellweg left a comment

mdellweg Feb 2, 2021

mdellweg Feb 2, 2021

bmbouter Feb 17, 2021

goosemania left a comment

goosemania commented Feb 2, 2021

mdellweg Feb 2, 2021

bmbouter Feb 2, 2021

mdellweg Feb 2, 2021

bmbouter commented Feb 2, 2021

bmbouter commented Feb 2, 2021

mdellweg commented Feb 2, 2021

mdellweg commented Feb 2, 2021

bmbouter commented Feb 3, 2021

mdellweg commented Feb 8, 2021

bmbouter commented Feb 8, 2021

mdellweg commented Feb 8, 2021

mdellweg Feb 17, 2021

mdellweg Feb 17, 2021

mdellweg commented Feb 17, 2021

bmbouter commented Feb 17, 2021

pulpbot commented Feb 23, 2021

	echo "component_name: pulp" >> .ci/ansible/vars/main.yaml
	echo "component_name: core" >> .ci/ansible/vars/main.yaml

Switches plugin reporting to use Django name #1102

Switches plugin reporting to use Django name #1102

Conversation

bmbouter commented Feb 1, 2021 • edited Loading

bmbouter commented Feb 1, 2021

mdellweg left a 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

goosemania left a comment

Choose a reason for hiding this comment

goosemania commented Feb 2, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bmbouter commented Feb 2, 2021

bmbouter commented Feb 2, 2021

mdellweg commented Feb 2, 2021

mdellweg commented Feb 2, 2021

bmbouter commented Feb 3, 2021

mdellweg commented Feb 8, 2021

bmbouter commented Feb 8, 2021

mdellweg commented Feb 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdellweg commented Feb 17, 2021

bmbouter commented Feb 17, 2021

pulpbot commented Feb 23, 2021

bmbouter commented Feb 1, 2021 •

edited

Loading