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

Removing pearson communication code #4765

Merged
merged 9 commits into from
Feb 5, 2021
Merged

Removing pearson communication code #4765

merged 9 commits into from
Feb 5, 2021

Conversation

annagav
Copy link
Contributor

@annagav annagav commented Jan 29, 2021

What are the relevant tickets?

Fix #4754

What's this PR do?

Remove legacy pearson code
Should also reomve the following config variables:

EXAMS_AUDIT_S3_BUCKET
EXAMS_AUDIT_AWS_ACCESS_KEY_ID
EXAMS_AUDIT_AWS_SECRET_ACCESS_KEY

How should this be manually tested?

nothing should break

@odlbot odlbot had a problem deploying to micromasters-ci-pr-4765 January 29, 2021 21:19 Failure
@codecov-io
Copy link

codecov-io commented Feb 1, 2021

Codecov Report

Merging #4765 (4f90ea7) into master (30afec9) will decrease coverage by 6.30%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4765      +/-   ##
==========================================
- Coverage   94.39%   88.09%   -6.31%     
==========================================
  Files         512      193     -319     
  Lines       23349     8230   -15119     
  Branches      967      902      -65     
==========================================
- Hits        22040     7250   -14790     
+ Misses       1206      885     -321     
+ Partials      103       95       -8     
Impacted Files Coverage Δ
exams/api.py 85.10% <ø> (+0.89%) ⬆️
...xams/management/commands/import_edx_exam_grades.py 0.00% <0.00%> (ø)
exams/signals.py 100.00% <ø> (ø)
exams/tasks.py 90.90% <ø> (-1.40%) ⬇️
exams/utils.py 100.00% <ø> (ø)
micromasters/settings.py 89.58% <ø> (-0.81%) ⬇️
micromasters/urls.py 80.00% <ø> (ø)
ui/views.py 93.54% <ø> (ø)
exams/constants.py 100.00% <100.00%> (ø)
grades/factories.py 100.00% <100.00%> (ø)
... and 297 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30afec9...4f90ea7. Read the comment docs.

@odlbot odlbot had a problem deploying to micromasters-ci-pr-4765 February 1, 2021 14:45 Failure
@odlbot odlbot temporarily deployed to micromasters-ci-pr-4765 February 1, 2021 15:48 Inactive
@pdpinch
Copy link
Member

pdpinch commented Feb 1, 2021

Can you change the typo in the PR title and your commits? It should be "pearson" with an "a". I'm just planning ahead for when we might have to search the repo for this change.

@annagav annagav changed the title removing person communication code Removing pearson communication code Feb 1, 2021
@annagav annagav force-pushed the ag/remove_paerson_code branch from e816ead to 45f9e26 Compare February 1, 2021 19:53
@annagav annagav temporarily deployed to micromasters-ci-pr-4765 February 1, 2021 19:53 Inactive
@annagav annagav temporarily deployed to micromasters-ci-pr-4765 February 1, 2021 20:15 Inactive
@rhysyngsun rhysyngsun self-assigned this Feb 3, 2021
Copy link
Contributor

@rhysyngsun rhysyngsun left a comment

Choose a reason for hiding this comment

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

Functionally looking good but a few extra things we should get rid of:

  • Move exams/pearson/constants.py to exams/constants.py
  • In exams/tasks.py remove the PEARSON_* constants
  • Remove unused pearson exam settings from settings.py
  • Remove unused pearson exam env var definitions from app.json

@annagav annagav temporarily deployed to micromasters-ci-pr-4765 February 3, 2021 17:42 Inactive
@rhysyngsun
Copy link
Contributor

@annagav looks like there's a test failure due to the removal of some settings.py stuff

@annagav annagav had a problem deploying to micromasters-ci-pr-4765 February 4, 2021 18:13 Failure
@annagav annagav had a problem deploying to micromasters-ci-pr-4765 February 4, 2021 21:01 Failure
@annagav annagav had a problem deploying to micromasters-ci-pr-4765 February 4, 2021 21:39 Failure
@annagav annagav had a problem deploying to micromasters-ci-pr-4765 February 4, 2021 22:10 Failure
@annagav
Copy link
Contributor Author

annagav commented Feb 4, 2021

@rhysyngsun I fixed the tests. Thank you!

@annagav annagav merged commit e83854e into master Feb 5, 2021
@annagav annagav deleted the ag/remove_paerson_code branch February 5, 2021 16:16
@odlbot odlbot mentioned this pull request Feb 5, 2021
2 tasks
@pdpinch pdpinch mentioned this pull request Mar 6, 2023
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove legacy pearson code
5 participants