Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: rename directories, "-" → "_" (for django 3) #176

Merged
merged 21 commits into from
Jul 18, 2023
Merged

Conversation

wesleyboar
Copy link
Member

@wesleyboar wesleyboar commented Apr 22, 2023

Status

These are each being tested during migration to https://github.com/TACC/Core-CMS-Custom.

I warn in README to not deploy sites via this repo using Core-CMS v3.12, and instead migrate.

Overview / Changes

Rename …-cms|org to …_cms|org.

Related

Testing & UI

See TACC/Core-CMS#626.

Notes

Without this PR, TACC/Core-CMS#625 build fails with:

[...] django.core.exceptions.ImproperlyConfigured: The app label 'example-cms' is not a valid Python identifier.

This update will break all Jenkins builds until edited. Also, I'd like to test these changes before merge. But I am willing to just bite the bullet and merge, then test each as they need to be re-deployed or as we have time. Fixes can always go into sites as they need them, and blame this PR as the origin of the problem.

If deployed on relevant sites, site pages will be missing template.

All pages need template re-assigned through CMS admin.
@wesleyboar wesleyboar changed the title feat: rename example-cms to example_cms feat: rename …-cms|org to …_cms|org (for django 3.2) May 1, 2023
@wesleyboar wesleyboar changed the title feat: rename …-cms|org to …_cms|org (for django 3.2) feat!: rename …-cms|org to …_cms|org (for django 3.2) May 3, 2023
@wesleyboar wesleyboar changed the title feat!: rename …-cms|org to …_cms|org (for django 3.2) feat!: rename all sites and static file directories ("-" → "_") May 3, 2023
@wesleyboar wesleyboar requested a review from taoteg May 3, 2023 19:38
@wesleyboar wesleyboar added the paused Started but not actively in progress label May 8, 2023
@wesleyboar wesleyboar changed the title feat!: rename all sites and static file directories ("-" → "_") feat!: rename directories, "-" → "_" (for django 3) May 23, 2023
@wesleyboar wesleyboar marked this pull request as draft May 24, 2023 14:22
@wesleyboar wesleyboar marked this pull request as ready for review May 24, 2023 19:33
Copy link
Member Author

Choose a reason for hiding this comment

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

Filenames do not matter.

The directories that matter are apps, and directories named identical to apps, like …/texascale_org and …/texascale_org/static/texascale_org

Copy link
Collaborator

@taoteg taoteg left a comment

Choose a reason for hiding this comment

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

All good except LCCF, where the subdir still has its hyphen in two files (see comments).
Not sure if the settings_custom need to be updated as well.
Otherwise, all good!
Approving.

@wesleyboar
Copy link
Member Author

Thanks, @taoteg.

I updated all the settings_custom.py a while back, but your comment prompted me to check again. I found some recent newcomers (from a merge of main) that I missed. I got 'em now.

I think what is next to do is test them all.

Copy link
Collaborator

@taoteg taoteg left a comment

Choose a reason for hiding this comment

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

Nice catch on brainmap!

@wesleyboar wesleyboar removed the paused Started but not actively in progress label May 24, 2023
@wesleyboar wesleyboar marked this pull request as draft June 22, 2023 19:14
@wesleyboar
Copy link
Member Author

Some CMS projects rely on templates/ specifically within the …-cms/ directory, so a rename alone will not be adequate.

  • To keep …-cms directories (even as symlinks) will ensure deploy fails with:
    The app label '…-cms' is not a valid Python identifier.
  • To remove …-cms directories will ensure CMS's pages have 500 Internal Server Error upon deploy.

I investigate alternatives.

@wesleyboar
Copy link
Member Author

wesleyboar commented Jun 22, 2023

Solutions

For any solution:

  1. ✓ rename all the project dirs
  2. → update CMS_TEMPLATES:
    • add new dir templates with same name as old dir templates
    • flag old dir templates as "DEPRECATED …"

Then continue with any chosen solution.

👎 A. Edit Pages to Use New Templates

  1. → create symlinks from old dirs to new dirs only for affected projects1
  2. soon, deploy to affected projects1
  3. soon, change the template of all affected pages2
  4. soon, remove symlinks
  5. soon, deploy to affected projects1

Untested.

✅ B. Add "Redirect" Templates

  1. → create old dirs only for affected projects1
  2. → in old dirs, add missing templates that only extend the new templates
  3. whenever, deploy to affected projects1
  4. whenever, change the template of all affected pages2

Tested on DemData.

Footnotes

  1. Projects with affected pages2 2 3 4 5

  2. Pages that load CMS_TEMPLATES from …-cms/templates 2 3

@wesleyboar
Copy link
Member Author

wesleyboar commented Jun 27, 2023

[I moved this status update to the PR description.]

@wesleyboar wesleyboar marked this pull request as ready for review July 18, 2023 19:59
@wesleyboar wesleyboar merged commit f6ef480 into main Jul 18, 2023
@wesleyboar wesleyboar deleted the django/2to3 branch July 18, 2023 20:00
@wesleyboar
Copy link
Member Author

I merged this so that CMS v3.12 can be released. All un-migrated sites (except TAPIS) are deployed with CMS v3.11. To avoid the problem and encourage migration, sites must not be deployed with CMS v3.12 unless migrated. I have stated such in the README.

wesleyboar added a commit to TACC/Core-CMS that referenced this pull request Jul 18, 2023
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.

2 participants