Skip to content

OSDOCS 8020 modify the build_for_portal.py and makeBuild.py scripts to support second-level books and improve intra-book links#65701

Merged
kalexand-rh merged 1 commit intoopenshift:mainfrom
mramendi:cicd-book-build
Oct 9, 2023
Merged

OSDOCS 8020 modify the build_for_portal.py and makeBuild.py scripts to support second-level books and improve intra-book links#65701
kalexand-rh merged 1 commit intoopenshift:mainfrom
mramendi:cicd-book-build

Conversation

@mramendi
Copy link
Contributor

@mramendi mramendi commented Oct 3, 2023

Version(s):

4.14+ to validate

Issue:

Link to docs preview:

QE review:

  • No QE is required for this change.

Additional information:

@openshift-ci openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 3, 2023
os.makedirs(directory)

def expand_huge_books(info):
"""
Copy link
Contributor

@maxwelldb maxwelldb Oct 4, 2023

Choose a reason for hiding this comment

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

Would something like this work if you wanted to go mad with list comprehensions?

def expand_huge_books(info):
"""
Finds nodes for huge books, creates new nodes for books from their top-level topics,
and then removes the nodes for huge books
"""
huge_book_nodes = [book for book in info["book_nodes"] if book["Name"] in LIST_OF_HUGE_BOOKS]
additional_nodes = [
    {
        "Dir": f"{book['Dir']}/{topic['Dir']}",
        **topic,
    }
    for book in huge_book_nodes
    for topic in book["Topics"]
    if "Dir" in topic
]
info["huge_book_dirs"].extend(book["Dir"] for book in huge_book_nodes)
info["book_nodes"] = [node for node in info["book_nodes"] if node not in huge_book_nodes]
info["book_nodes"].extend(additional_nodes)

Could also consider pathlib for all of these changes if you like its ergonomics. 🤷

⚠️ I've only had green tea this morning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure about going just this mad with list comprehensions but I will try to use some of this tomorrow Thursday. As for pathlib, I think it should be used everywhere or nowhere and it's to much work to use it everywhere.

@mramendi mramendi changed the title WIP: Add CI/CD to list of huge books in build_for_portal.py OSDOCS 8020 modify the build_for_portal.py and makeBuild.py scripts to support second-level books and improve intra-book links Oct 4, 2023
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 4, 2023
…o support second-level books and improve intra-book links
@kalexand-rh
Copy link
Contributor

@maxwelldb, do you see any reason to wait to merge this one?

@maxwelldb
Copy link
Contributor

@kalexand-rh Not really, no.

It'd be nice to run Black on any Python code as a matter of neatness, but you're the DPM and can make a call as to whether you want to be strict about formatting or not. 👍

@kalexand-rh
Copy link
Contributor

Then for the sake of orderliness, will one you do that?

@mramendi
Copy link
Contributor Author

mramendi commented Oct 9, 2023

@maxwelldb while I don't have a problem with running Black, I suggest this be done separately, just in case Black alters the execution even though it is not designed to do so.

There is one more issue to work out anyway: the imp module, which is deprecated and slated to be removed. I can look into this - but again I'd like to separate this from the function enhancement.

@kalexand-rh
Copy link
Contributor

Fair enough. I'm going to merge it and CP it to 4.14.

@kalexand-rh kalexand-rh merged commit cda9024 into openshift:main Oct 9, 2023
@kalexand-rh
Copy link
Contributor

/cherrypick enterprise-4.14

@openshift-cherrypick-robot

@kalexand-rh: #65701 failed to apply on top of branch "enterprise-4.14":

Applying: OSDOCS 8020 modify the build_for_portal.py and makeBuild.py scripts to support second-level books and improve intra-book links
Using index info to reconstruct a base tree...
M	build_for_portal.py
Falling back to patching base and 3-way merge...
Auto-merging build_for_portal.py
CONFLICT (content): Merge conflict in build_for_portal.py
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 OSDOCS 8020 modify the build_for_portal.py and makeBuild.py scripts to support second-level books and improve intra-book links
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

Details

In response to this:

/cherrypick enterprise-4.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@kalexand-rh
Copy link
Contributor

@mramendi, will you please manually cherry-pick to 4.14? I reviewed the commit history between main and enterprise-4.14. Usually CPs from main to the branch for the version that's about to be released need to go cleanly, but I think this is one instance where it will make more sense to manually CP.

@mramendi
Copy link
Contributor Author

mramendi commented Oct 9, 2023

@kalexand-rh done #65981

@aireilly
Copy link
Contributor

aireilly commented Oct 16, 2023

This change passes broken AsciiDoc and reports a passed build. IMO it should be reverted, reworked, and tested thoroughly before merging.

@mramendi
Copy link
Contributor Author

@aireilly details please?

@aireilly
Copy link
Contributor

aireilly commented Oct 16, 2023

All 4.14 portal validation builds that run makeBuild.py and build.py pass the build even when there are errors in the build. Example: https://app.travis-ci.com/github/openshift/openshift-docs/jobs/611584490

This means that the 4.14 production build for the portal OCP docs is now broken.

@kalexand-rh
Copy link
Contributor

The sync to the portal uses build_for_portal.py, not build.py, and, at least as of Thursday, all of the 4.14 PV1 jobs were passing. I think we need to investigate switching Travis to use build_for_portal.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

branch/enterprise-4.14 size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants