Skip to content
This repository was archived by the owner on May 6, 2024. It is now read-only.

chore!: removing lx-pathway-plugin from private requirements - #7030

Merged
awais786 merged 3 commits into
masterfrom
remove-ls-pathway
Oct 18, 2023
Merged

chore!: removing lx-pathway-plugin from private requirements#7030
awais786 merged 3 commits into
masterfrom
remove-ls-pathway

Conversation

@awais786

@awais786 awais786 commented Oct 2, 2023

Copy link
Copy Markdown
Contributor

This Package is no more in use. Need to remove this or update due to django42 compatiblity.

Since mysql8 backup still going on. So instead of deleting table i am removing this package from installation. After backup completion will remove the tables completely.

conversation about removing/deleting tables open-craft/lx-pathway-plugin#1 (comment)

They merged PR with deletion table migration.

@awais786 awais786 changed the title chore: temp removing this to test building sandbox with django42. chore: removing lx-path from requirements. Oct 2, 2023
@awais786
awais786 marked this pull request as ready for review October 2, 2023 14:11
@connorhaugh

connorhaugh commented Oct 2, 2023

Copy link
Copy Markdown
Contributor

@awais786
I assume you are doing this removal work so you don't have to do the django 4.2 upgrade for this requirement.

Your current plan would be:

  1. Remove this requirement
  2. Do the django 4.2 upgrade for edx-platform
  3. wait for mysql 8 stuff to be done
  4. Upgrade the django to 4.2 for lx-pathways
  5. Re add the requirement @ version '3.0.4'
  6. Re-remove the requirement.

I imagine steps 3-6 would never be done, and we would never remove the tables, or SRE would have to do it.
I'm unsure why the myql 8 work precludes us from doing this migration. Instead, I think we should:

  1. upgrade lx-pathways @ version '3.0.4' to drop the tables
  2. Remove the requirement.

Let me know if you disagree.

@adzuci
adzuci requested a review from jmbowman October 2, 2023 15:51
adzuci
adzuci previously requested changes Oct 2, 2023

@adzuci adzuci left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@awais786 Could we follow the normal process here to avoid setting a poor precedent even though we are confident this code is not in use? https://openedx.atlassian.net/wiki/spaces/COMM/pages/1051361376/Removing+a+Djangoapp+from+an+existing+project

@adzuci
adzuci requested a review from connorhaugh October 2, 2023 15:52
Comment thread playbooks/roles/edxapp/defaults/main.yml Outdated
Comment thread playbooks/roles/edxapp/defaults/main.yml Outdated
@awais786 awais786 closed this Oct 9, 2023
@awais786 awais786 reopened this Oct 9, 2023
@awais786

Copy link
Copy Markdown
Contributor Author

@connorhaugh its ready to go.

@awais786
awais786 requested a review from adzuci October 12, 2023 12:44

@UsamaSadiq UsamaSadiq left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verified the commit hash to be the latest master hash from the repository.

@awais786

Copy link
Copy Markdown
Contributor Author

@adzuci your requested changes implemented.

@iamsobanjaved iamsobanjaved changed the title chore: removing lx-path from requirements. chore!: removing lx-pathway-plugin from private requirements Oct 17, 2023

@connorhaugh connorhaugh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good thank you for following the safest process here!

@awais786
awais786 merged commit f540b55 into master Oct 18, 2023
@awais786
awais786 deleted the remove-ls-pathway branch October 18, 2023 12:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants