-
Notifications
You must be signed in to change notification settings - Fork 7
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
Ingest sloan courses via api #1487
base: main
Are you sure you want to change the base?
Conversation
# Conflicts: # src/ol_orchestrate/resources/openedx.py
def get_sloan_courses(self): | ||
""" | ||
Retrieve the course data from their API as JSON | ||
returns: JSON document representing an array of course objects | ||
""" | ||
course_url = "https://mit-unified-portal-prod-78eeds.43d8q2.usa-e2.cloudhub.io/api/courses" | ||
return self._fetch_with_auth(course_url) | ||
|
||
def get_sloan_course_offerings(self): | ||
""" | ||
Retrieve the course offerings data from their API as JSON | ||
returns: JSON document representing an array of course offering objects | ||
""" | ||
course_offering_url = "https://mit-unified-portal-prod-78eeds.43d8q2.usa-e2.cloudhub.io/api/course-offerings" | ||
return self._fetch_with_auth(course_offering_url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These method seem like they're unnecessary in this context. If we wanted to make a Sloan API specific resource we could put them there, but I think it's easy enough to just use the fetch_with_auth
and pass the URL in question. Another reason to not have these methods here is that they will then be inherited by the edX resource (which won't break anything, but is a leaky abstraction)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I realized that they don't belong here. I moved them to the asset file so we don't need to create a new resource file for these two simple methods.
What are the relevant tickets?
https://github.com/mitodl/hq/issues/6687
Description (What does it do?)
Creating new dagster code location, definition, and assets for learning resources such as Sloan courses.
Also, creating a new OAuthApiClient resource that is not tied to the edx instance, and modifying OpenEdxApiClient to inherit from it
How can this be tested?
export GITHUB_TOKEN=
docker compose up
Go to http://127.0.0.1:3000/locations/ol_orchestrate.definitions.learning_resource.extract_api_data/assets, ensure 2 assets listed.
Click on Materialize for the two assets
The existing assets that use OpenEdxApiClient should still work. This could be tested by materializing any of http://127.0.0.1:3000/locations/ol_orchestrate.definitions.edx.retrieve_edxorg_raw_data/assets