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.

Sign up for GitHub

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

Jump to bottom

Move user retirement scripts code from the tubular repo #34063

Merged
merged 10 commits into from
Feb 22, 2024
Merged

Move user retirement scripts code from the tubular repo #34063

merged 10 commits into from
Feb 22, 2024

Conversation

farhan
Copy link
Contributor

@farhan farhan commented Jan 16, 2024 •
edited
Loading

Ticket: openedx/axim-engineering#881

Description:
Move user retirement scripts code from the tubular repo into this repo.

Script readme and how to docs:
https://github.com/farhan/edx-platform/blob/farhan/code-migrated-from-tubular/scripts/user_retirement/README.rst

Relevent removal PR in tubular repo
openedx-unsupported/tubular#734

@farhan farhan changed the title refactor: Repo adopting user retirement scripts code from the tubular repo refactor: Move user retirement scripts code from the tubular repo Jan 16, 2024
@farhan farhan force-pushed the farhan/code-migrated-from-tubular branch 21 times, most recently from 8da6258 to d491961 Compare January 24, 2024 05:44
@farhan farhan changed the title refactor: Move user retirement scripts code from the tubular repo Move user retirement scripts code from the tubular repo Jan 24, 2024
@farhan farhan marked this pull request as ready for review January 25, 2024 12:48
@farhan farhan mentioned this pull request Jan 25, 2024
Closed
7 tasks
@farhan farhan requested a review from bmtcril January 30, 2024 06:12
bmtcril
bmtcril reviewed Feb 1, 2024
View reviewed changes
Copy link
Contributor

@bmtcril bmtcril left a comment

Choose a reason for hiding this comment

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

Just a couple of small comments, otherwise this looks good! I think there are some longer term issues around supporting specific 3rd party APIs, but this probably isn't the place to tease that apart.

Do you know if this move has been communicated to 2U and the community? If we deprecate Tubular it may break existing retirement pipelines for a lot of people.

scripts/user_retirement/utils/google_api.py Outdated Show resolved Hide resolved
.github/workflows/units-test-scripts-user-retirement.yml Outdated Show resolved Hide resolved
@farhan
Copy link
Contributor Author

farhan commented Feb 6, 2024

@bmtcril Thanks for the review. PR is ready for the next pass

Do you know if this move has been communicated to 2U and the community? If we deprecate Tubular it may break existing retirement pipelines for a lot of people.

@feanil is taking care of it,
perhaps 2U can maintain it (avoiding deprecation) and they will own it all now.

@farhan farhan requested a review from feanil February 7, 2024 14:56
@bmtcril bmtcril mentioned this pull request Feb 8, 2024
Open
2 tasks
@salman2013
Copy link
Contributor

@farhan there is one more script mentioned in the ticket structures.py. Is there any reason why it is not moved?

@farhan
Copy link
Contributor Author

farhan commented Feb 15, 2024

@farhan there is one more script mentioned in the ticket structures.py. Is there any reason why it is not moved?

@salman2013 As per recommendation here. I didn't move it.

salman2013
salman2013 approved these changes Feb 15, 2024
View reviewed changes
Copy link
Contributor

@salman2013 salman2013 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@farhan farhan mentioned this pull request Feb 17, 2024
Merged
feanil
feanil approved these changes Feb 21, 2024
View reviewed changes
Copy link
Contributor

@feanil feanil left a comment

Choose a reason for hiding this comment

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

@farhan changes generally look good to me. One ToDo for tickting up some follow up work. Let me know when that ticket exists and I can merge this code.

scripts/user_retirement/README.rst Show resolved Hide resolved
@feanil feanil mentioned this pull request Feb 21, 2024
Closed
@farhan farhan mentioned this pull request Feb 22, 2024
Closed
@farhan farhan force-pushed the farhan/code-migrated-from-tubular branch from d19b7b6 to 4a139f7 Compare February 22, 2024 06:00
@farhan
Copy link
Contributor Author

farhan commented Feb 22, 2024

@feanil PR is ready to merge. Please Squash and Merge

@feanil feanil merged commit 65ea55c into openedx:master Feb 22, 2024
47 checks passed
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@feoh feoh mentioned this pull request Feb 22, 2024
Open
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

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

@bmtcril bmtcril bmtcril left review comments

@feanil feanil feanil approved these changes

@salman2013 salman2013 salman2013 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@farhan @salman2013 @edx-pipeline-bot @feanil @bmtcril