ETL pipeline for MIT edX programs #1536
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rhysyngsun
requested changes
Sep 10, 2024
Comment on lines
21
to
27
| Helper function to determine if a course is an MIT course | ||
| Args: | ||
| course (dict): The JSON object representing the course with all its course runs | ||
| Returns: | ||
| bool: indicates whether the course is owned by MIT |
Contributor
There was a problem hiding this comment.
Docstring needs to be updated from copy/paste.
| # use the OpenEdx factory to create our extract and transform funcs | ||
| extract, _transform = openedx_extract_transform_factory(get_open_edx_config) | ||
|
|
||
| # modified transform function that filters the course list to ones that pass the _is_mit_course() predicate # noqa: E501 |
Contributor
There was a problem hiding this comment.
Update comment referencing wrong function
learning_resources/etl/openedx.py
Outdated
Comment on lines
375
to
381
| Transform a course run into the normalized data structure | ||
| Args: | ||
| config (OpenEdxConfiguration): configuration for the openedx backend | ||
| Returns: | ||
| dict: the tranformed course run data |
learning_resources/etl/openedx.py
Outdated
| return dates | ||
|
|
||
|
|
||
| def _add_course_prices(program): |
Contributor
There was a problem hiding this comment.
I'd suggest changing the name of this function since _add_course_prices sounds like it'd add the course prices to the program object being passed in. Replacing "add" with "sum" is probably good enough.
learning_resources/tasks.py
Outdated
| programs = pipelines.mit_edx_programs_etl(api_datafile) | ||
| clear_search_cache() | ||
| return len(courses) | ||
| return len(courses + programs) |
Contributor
There was a problem hiding this comment.
Nit: it's probably a bit better to take len(courses) + len(programs) so that you're not generating an entirely new list in memory just to take the len() of it.
mbertrand
force-pushed
the
mb/mit_edx_programs
branch
from
6e09db4 to
262ea10
Compare
Member
Author
|
@rhysyngsun ready for another look |
mbertrand
force-pushed
the
mb/mit_edx_programs
branch
from
262ea10 to
1e4f1bb
Compare
mbertrand
force-pushed
the
mb/mit_edx_programs
branch
from
1e4f1bb to
e504a81
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are the relevant tickets?
Closes https://github.com/mitodl/hq/issues/5389
Description (What does it do?)
Adds an ETL pipeline for ingesting MIT edX programs from an API endpoint
Screenshots (if appropriate):
How can this be tested?
./manage.py backpopulate_edx_dataAdditional Context
The programs API endpoint also returns micromasters programs, which are currently ignored in favor of the data from the micromasters API endpoint.