Move docs test workflow and rm obsolete docs processes #902

pedro-psb · 2024-07-26T15:31:38Z

This does:

Adds a docs test workflow in the ci.yml
Removes docs test from test.yml workflow (which tests the plugin)
Deprecate --docs feature from plugin-template
- remove docs_test option
- remove template/docs
- remove template/bootstrap/docs legacy content
- remove ci_update_docs
- make it no-ops
Update various info across files

Closes: #901

Testing

This does not cover all workflows, only CI

Run in a sample PR: https://github.com/pedro-psb/pulp_python/pull/2/checks
Merging to non-main branch: https://github.com/pedro-psb/pulp_python/pull/3/checks
Cache test: Not very meaningful time saving. Around 30mb of cached data
- no-cache run
- cached run

templates/github/.github/workflows/ci.yml.j2

This does: * Adds a docs test workflow in the ci.yml * Removes docs test from test.yml workflow (which tests the plugin) * Deprecate --docs feature from plugin-template * remove docs_test option * remove template/docs * remove template/bootstrap/docs legacy content * remove ci_update_docs * make it no-ops * Update various info across files Closes: pulp#901

pedro-psb · 2024-07-26T21:38:31Z

templates/github/.github/workflows/docs.yml.j2

+    runs-on: "ubuntu-20.04"
+    steps:
+      - run: |
+          echo "Skip docs testing on non-main branches."


I tried moving the skip to the ci.yml and got some trouble with the final "ready-to-ship" checks.
I could update the logic there to allow skipping docs (I can, if that sounds better), but this approach feels simpler after all.

I think I ran into the exact same issues before. Still anyone with a better idea...

mdellweg · 2024-07-29T08:33:20Z

templates/github/.github/workflows/scripts/script.sh.j2

 # Gets set in .github/settings.yml, but doesn't seem to inherited by
 # this script.
 export DJANGO_SETTINGS_MODULE=pulpcore.app.settings
 export PULP_SETTINGS=$PWD/.ci/ansible/settings/settings.py

 export PULP_URL="{{ pulp_scheme }}://pulp"

-if [[ "$TEST" = "docs" ]]; then
-  if [[ "$GITHUB_WORKFLOW" == "{{ plugin_app_label | camel | default("Pulp") }} CI" ]]; then
-    towncrier build --yes --version 4.0.0.ci


I wish we could keep this command somewhere.

The towncrier build command?
On a first sight I just though it was not strictly related to the docs building process, but I can see we want to test if the PR contain valid assets for generating the changelogs.

I'll add a step in the new docs.yml called Build changelog, wdyt?

That sounds great. We should be using the generated changelog in the markdown build (or at least do a syntax check).

[noissue]

mdellweg · 2024-07-30T08:49:36Z

templates/github/.github/workflows/docs.yml.j2

@@ -28,7 +28,10 @@ jobs:
          key: {{ "${{ runner.os }}-pip-${{ hashFiles('pulp-docs-main-sha') }}" }}
          restore-keys: |
            {{ "${{ runner.os }}-pip-" }}
-      {{ install_python_deps(["-r", "doc_requirements.txt"]) | indent(6) }}
+      {{ install_python_deps(["-r", "doc_requirements.txt", "towncrier"]) | indent(6) }}


I'm inclined to ask to just make it a doc_requirement...

mdellweg · 2024-07-30T08:52:35Z

templates/bootstrap/doc_requirements.txt.j2

@@ -1,5 +1,2 @@
 {% include 'header.j2' %}
-r requirements.txt
-towncrier


Ohh, It was one... ;)

Yeah, makes sense with having the changelog build step.
Will add towncrier back to doc_requirements.

mdellweg · 2024-07-30T08:54:23Z

templates/github/.github/workflows/ci.yml.j2

+  docs:
+    uses: "./.github/workflows/docs.yml"
+


We should keep this behind a flag, because a plugin outside of our control may want to do docs different.

How do you think we should determine the docs-managed repos?

fetching from the existing config file in pulp-docs?

declaring a list here?

other?

Oh, I see your point. So the one point of authoritative information is the pulp-docs repository, yes?
Then we should take the information from there.

[noissue]

templates/bootstrap/docs/index.md.j2

templates/github/.github/workflows/ci.yml.j2

mdellweg · 2024-08-02T08:48:46Z

templates/github/.github/workflows/scripts/install.sh.j2

 {%- if plugin_name == 'pulpcore' %}
+if [[ "$TEST" = "publish" ]]


This sounds ridiculous. There is no "publish" test.

It's safe to remove this, then?

I believe so. Maybe start with a "git grep publish". But I'm pretty sure this is a leftover from former versions of the release process.

mdellweg · 2024-08-02T08:51:20Z

templates/github/.github/workflows/scripts/install.sh.j2

 pulp config create --base-url {{ pulp_scheme }}://pulp{% if pulp_scheme != 'https' %} --no-verify-ssl{% endif %} --api-root "$PULP_API_ROOT" --username "admin" --password "password"
 {% if test_cli -%}
-if [[ "$TEST" != "docs" ]]; then
-  cp ~/.config/pulp/cli.toml "${REPO_ROOT}/../{{ cli_package }}/tests/cli.toml"
-fi
+cp ~/.config/pulp/cli.toml "${REPO_ROOT}/../{{ cli_package }}/tests/cli.toml"
 {%- endif %}


Can you move that whole block up to around line 30 to have the cli stuff in one place?

Didn't seem to work. Don't try this again just now.

mdellweg · 2024-08-02T08:53:15Z

requirements.txt

@@ -1,2 +1,3 @@
 tomlkit~=0.12.4
 pypandoc_binary~=1.13


looks like this can be dropped again.

pedro-psb · 2024-08-02T16:10:33Z

templates/github/.github/workflows/scripts/install.sh.j2

 {% if test_cli -%}
+cp ~/.config/pulp/cli.toml "${REPO_ROOT}/../{{ cli_package }}/tests/cli.toml"
 PULP_CLI_VERSION="$(pip freeze | sed -n -e 's/{{ cli_package }}==//p')"
 git clone --depth 1 --branch "$PULP_CLI_VERSION" {{ cli_repo }} ../{{ cli_package }}
 {%- endif %}


Hmm, I've just reviewed that move, around line 138 we have:

{%- if test_reroute %} export PULP_API_ROOT="/rerouted/djnd/" {%- endif %}

So I guess it really should be down there, so it can use the proper value of PULP_API_ROOT.

Hmm. that explains why we do it down there.
Let's not do this now then. But we should think about a more structured approach to set all this up in a later PR.

Co-authored-by: Matthias Dellweg <2500@gmx.de> [noissue]

mdellweg

This is a huge change. And maybe we can iterate to improve on it. But not necessarily in this PR.

Things to follow up on:

A more structured approach to define pulp settings, configure the cli, start the containers, ...
Are there we should mark as deprecated from the old docs templates? Is CI now the right place to delete them?

mdellweg · 2024-08-05T14:32:09Z

templates/github/.github/workflows/docs.yml.j2

+    runs-on: "ubuntu-20.04"
+    steps:
+      - run: |
+          echo "Skip docs testing on non-main branches."


I think I ran into the exact same issues before. Still anyone with a better idea...

mdellweg · 2024-08-05T14:33:16Z

templates/github/.github/workflows/scripts/install.sh.j2

 {%- if plugin_name == 'pulpcore' %}
+if [[ "$TEST" = "publish" ]]


I believe so. Maybe start with a "git grep publish". But I'm pretty sure this is a leftover from former versions of the release process.

mdellweg · 2024-08-05T14:34:06Z

templates/github/.github/workflows/scripts/install.sh.j2

 pulp config create --base-url {{ pulp_scheme }}://pulp{% if pulp_scheme != 'https' %} --no-verify-ssl{% endif %} --api-root "$PULP_API_ROOT" --username "admin" --password "password"
 {% if test_cli -%}
-if [[ "$TEST" != "docs" ]]; then
-  cp ~/.config/pulp/cli.toml "${REPO_ROOT}/../{{ cli_package }}/tests/cli.toml"
-fi
+cp ~/.config/pulp/cli.toml "${REPO_ROOT}/../{{ cli_package }}/tests/cli.toml"
 {%- endif %}


Didn't seem to work. Don't try this again just now.

mdellweg · 2024-08-05T14:36:42Z

utils.py

-    )
-    document = header + document
-    return document
+    return [line.strip()[8:] for line in response.content.decode().split("\n") if "- name:" in line]


This is a rather crude way to parse yaml...

My plan is to do the minimum working solution now and later improve this membership thing (on pulp-docs, also). But I can add a yml library as requirement and make it more robust. Would you prefer that?

Will it stay parsing yaml? Or did you plan something completely different?

I thought about doing a simple requirement-like file which lists gh-org/gh-repo and has # comments, but not sure if it's a good idea yet. If not, I might stick with yaml.

mdellweg · 2024-08-05T14:38:41Z

plugin-template

@@ -328,26 +327,25 @@ def main():
    except Exception:
        config["current_version"] = "0.1.0a1.dev"

+    # Determine if plugin is a member of Pulp managed documentation
+    config["is_pulpdocs_member"] = config["plugin_name"] in utils.get_pulpdocs_members()


I like it. When onboarding a new plugin to the docs, we need to be extra careful. (Just a reminder at this point.)

mdellweg · 2024-08-06T16:45:53Z

Let's merge and continue from there.

pedro-psb commented Jul 26, 2024

View reviewed changes

templates/github/.github/workflows/ci.yml.j2 Show resolved Hide resolved

pedro-psb force-pushed the move-docs-workflow-out-of-tests branch 10 times, most recently from 93acd3b to 65aebe3 Compare July 26, 2024 21:12

pedro-psb force-pushed the move-docs-workflow-out-of-tests branch from 65aebe3 to 65c470f Compare July 26, 2024 21:25

pedro-psb commented Jul 26, 2024

View reviewed changes

mdellweg reviewed Jul 29, 2024

View reviewed changes

Add "Build changelog" step to docs workflow

92cceaa

[noissue]

github-actions bot added the multi-commit label Jul 29, 2024

mdellweg reviewed Jul 30, 2024

View reviewed changes

pedro-psb added 2 commits August 1, 2024 17:47

Make plugin-template aware of pulp-docs memberships

fb9afd4

[noissue]

Remove changelog-migration script

b9ec40e

[noissue]

pedro-psb force-pushed the move-docs-workflow-out-of-tests branch from 469d8c8 to b9ec40e Compare August 1, 2024 20:55

pedro-psb requested a review from mdellweg August 1, 2024 21:18

mdellweg reviewed Aug 2, 2024

View reviewed changes

templates/bootstrap/docs/index.md.j2 Outdated Show resolved Hide resolved

mdellweg reviewed Aug 2, 2024

View reviewed changes

templates/github/.github/workflows/ci.yml.j2 Outdated Show resolved Hide resolved

mdellweg reviewed Aug 2, 2024

View reviewed changes

pedro-psb force-pushed the move-docs-workflow-out-of-tests branch from 86eb27e to 14d4ee7 Compare August 2, 2024 16:05

pedro-psb commented Aug 2, 2024

View reviewed changes

Review changes

a3e92bc

Co-authored-by: Matthias Dellweg <2500@gmx.de> [noissue]

pedro-psb force-pushed the move-docs-workflow-out-of-tests branch from 14d4ee7 to a3e92bc Compare August 5, 2024 13:25

mdellweg approved these changes Aug 5, 2024

View reviewed changes

mdellweg merged commit 31fae26 into pulp:main Aug 6, 2024
11 checks passed

pedro-psb deleted the move-docs-workflow-out-of-tests branch August 6, 2024 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move docs test workflow and rm obsolete docs processes #902

Move docs test workflow and rm obsolete docs processes #902

pedro-psb commented Jul 26, 2024 •

edited

Loading

pedro-psb Jul 26, 2024

mdellweg Aug 5, 2024

mdellweg Jul 29, 2024

pedro-psb Jul 29, 2024

mdellweg Jul 30, 2024

mdellweg Jul 30, 2024

mdellweg Jul 30, 2024

pedro-psb Jul 30, 2024

mdellweg Jul 30, 2024

pedro-psb Jul 30, 2024

mdellweg Jul 31, 2024

mdellweg Aug 2, 2024

pedro-psb Aug 2, 2024

mdellweg Aug 5, 2024

mdellweg Aug 2, 2024

mdellweg Aug 5, 2024

mdellweg Aug 2, 2024

pedro-psb Aug 2, 2024

mdellweg Aug 5, 2024 •

edited

Loading

pedro-psb Aug 5, 2024

mdellweg left a comment

mdellweg Aug 5, 2024

mdellweg Aug 5, 2024

mdellweg Aug 5, 2024

mdellweg Aug 5, 2024

pedro-psb Aug 5, 2024

mdellweg Aug 6, 2024

pedro-psb Aug 6, 2024

mdellweg Aug 5, 2024

mdellweg commented Aug 6, 2024

		{%- if plugin_name == 'pulpcore' %}
		if [[ "$TEST" = "publish" ]]

Move docs test workflow and rm obsolete docs processes #902

Move docs test workflow and rm obsolete docs processes #902

Conversation

pedro-psb commented Jul 26, 2024 • edited Loading

Testing

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

mdellweg Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

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

mdellweg commented Aug 6, 2024

pedro-psb commented Jul 26, 2024 •

edited

Loading

mdellweg Aug 5, 2024 •

edited

Loading