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

feat: deprecate learner view code #571

Merged
merged 2 commits into from
Jul 28, 2022
Merged

Conversation

alangsto
Copy link
Contributor

@alangsto alangsto commented Jul 26, 2022

Remove all code related to learner specific data. This includes

  • any code that involved elastic search
  • learner view endpoints
  • learner serializers
  • learner view tests

@alangsto alangsto force-pushed the alangsto/remove_learner_view branch 8 times, most recently from c051f29 to d30e8ff Compare July 26, 2022 20:26
@codecov
Copy link

codecov bot commented Jul 26, 2022

Codecov Report

Merging #571 (6899a05) into master (406b3c1) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #571      +/-   ##
==========================================
+ Coverage   97.48%   97.52%   +0.03%     
==========================================
  Files          63       54       -9     
  Lines        3944     3026     -918     
  Branches      554      412     -142     
==========================================
- Hits         3845     2951     -894     
+ Misses         68       63       -5     
+ Partials       31       12      -19     
Impacted Files Coverage Δ
analytics_data_api/middleware.py 95.58% <ø> (-4.42%) ⬇️
analytics_data_api/v0/apps.py 100.00% <ø> (+17.64%) ⬆️
analytics_data_api/v0/exceptions.py 92.85% <ø> (-7.15%) ⬇️
analytics_data_api/v0/urls/learners.py 100.00% <ø> (ø)
analytics_data_api/v0/views/__init__.py 96.49% <ø> (-0.57%) ⬇️
analytics_data_api/v0/models.py 99.44% <100.00%> (-0.08%) ⬇️
analytics_data_api/v0/serializers.py 93.03% <100.00%> (-1.97%) ⬇️
analytics_data_api/v0/tests/views/__init__.py 98.93% <100.00%> (-0.20%) ⬇️
analytics_data_api/v0/views/learners.py 100.00% <100.00%> (+3.53%) ⬆️
... and 2 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 406b3c1...6899a05. Read the comment docs.

@alangsto alangsto force-pushed the alangsto/remove_learner_view branch 8 times, most recently from f5b8ed6 to 19990cb Compare July 27, 2022 13:48
@@ -1,28 +1,6 @@
from django.apps import AppConfig
from django.conf import settings
from elasticsearch_dsl import connections
Copy link
Contributor

Choose a reason for hiding this comment

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

can we strip elasticsearch support out of requirements?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just ripped it out!

@@ -134,24 +95,6 @@ def status_code(self):
return status.HTTP_400_BAD_REQUEST


class ParameterValueErrorMiddleware(BaseProcessErrorMiddleware):
Copy link
Contributor

Choose a reason for hiding this comment

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

I like how you've found and smashed these associated classes

'number_of_replicas': 0
}
ELASTICSEARCH_INDEX_SETTINGS = environ.get('ELASTICSEARCH_INDEX_SETTINGS', DEFAULT_ELASTICSEARCH_INDEX_SETTINGS)
########## END ELASTICSEARCH CONFIGURATION
Copy link
Contributor

Choose a reason for hiding this comment

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

so satisfying

@ashultz0
Copy link
Contributor

I'm pretty heartened that you were able to get rid of the elasticsearch stuff even without removing module engagement

@alangsto alangsto force-pushed the alangsto/remove_learner_view branch 2 times, most recently from ad65019 to a27eb62 Compare July 27, 2022 17:12
@alangsto alangsto force-pushed the alangsto/remove_learner_view branch from a27eb62 to 6899a05 Compare July 27, 2022 17:14
@alangsto alangsto marked this pull request as ready for review July 27, 2022 17:24
@alangsto alangsto merged commit f8ca303 into master Jul 28, 2022
@alangsto alangsto deleted the alangsto/remove_learner_view branch July 28, 2022 12:42
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.

2 participants