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

refactor: remove X-EdX-Api-Key usage #104

Merged
merged 4 commits into from
May 22, 2024
Merged

refactor: remove X-EdX-Api-Key usage #104

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
351d151
refactor: remove X-EdX-Api-Key usage
mudassir-hafeez May 14, 2024
263c704
fix: update deactivate_enrollment API to remove X-EdX-Api-Key usage
mudassir-hafeez May 16, 2024
f819adc
fix: ci build fails for Python 3.5 setup with an invalid certificate …
mudassir-hafeez May 17, 2024
411a2c6
chore: remove fail_ci_if_error check
mudassir-hafeez May 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ jobs:
uses: actions/setup-python@v2
with:
python-version: "3.5"
env:
PIP_TRUSTED_HOST: "pypi.python.org pypi.org files.pythonhosted.org"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change fixes Python 3.5 setup fails with an invalid certificate when running pip. It's reported in actions/setup-python: actions/setup-python#866

- id: cache
uses: actions/cache@v1
Expand All @@ -32,4 +34,3 @@ jobs:
uses: codecov/codecov-action@v1
with:
file: ./coverage.xml
fail_ci_if_error: true
Copy link
Contributor Author

@mudassir-hafeez mudassir-hafeez May 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Removed this check for now as it is hitting a rate limit on Codecov and causing a CI build failure. To bypass this limit, we need to use a Codecov repository upload token.

This issue also exists in other repositories. You can see the build step Upload coverage to CodeCov failing on the master branches, e.g., mitxpro and mitxonline.

Could someone from the DevOps team take a look into this?

@blarghmatey FYI

Copy link
Contributor

Choose a reason for hiding this comment

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

@mudassir-hafeez Upgrading codecov/codecov-action@v1 to codecov/codecov-action@v3 might fix this. Could you check that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@asadali145 It seems upgrading to codecov/codecov-action@v3 did not resolve the issue. I've noticed this issue is being faced by multiple users with v3 as well. And Codecov team is suggesting moving on v4 with Codecov token.

1 change: 0 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ Your edx demo course (course id `course-v1:edX+DemoX+Demo_Course`)
must be enabled for CCX.

The Course Enrollment integration tests have the following requirements:
- The `API_KEY` value must match the `settings.EDX_API_TOKEN` value in LMS
- The user assigned to that `ACCESS_TOKEN` must be an admin in the edX demo course.
Adding a user as an admin can be done in Studio (url: `<studio_url>/course_team/course-v1:edX+DemoX+Demo_Course`

Expand Down
3 changes: 1 addition & 2 deletions edx_api/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,7 @@ def get_requester(self, token_type="Bearer"):
{
"Authorization": "{} {}".format(
token_type, self.credentials["access_token"]
),
"X-EdX-Api-Key": self.credentials.get("api_key"),
)
}
)

Expand Down
4 changes: 1 addition & 3 deletions edx_api/client_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,5 @@ def test_request_id_credential():
def test_instantiation_happypath():
"""instantiatable with correct args"""
token = 'asdf'
api_key = 'api_key'
client = EdxApi({'access_token': token, 'api_key': api_key})
client = EdxApi({'access_token': token})
assert client.get_requester().headers['Authorization'] == 'Bearer {token}'.format(token=token)
assert client.get_requester().headers['X-EdX-Api-Key'] == api_key
17 changes: 10 additions & 7 deletions edx_api/enrollments/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,24 +201,27 @@ def create_verified_student_enrollment(self, course_id, username=None):
username=username
)

def deactivate_enrollment(self, course_id):
def deactivate_enrollment(self, course_id, username=None):
"""
Deactivates an enrollment in the given course for the user
whose access token was provided to the API client (in other words, the
user will be unenrolled)

Args:
course_id (str): An edX course id.
username (str): Username.

Returns:
Enrollment: object representing the deactivated student enrollment
"""

params = {
"course_details": {"course_id": course_id},
"is_active": False,
}
if username:
params['user'] = username
resp = self.requester.post(
urljoin(self.base_url, self.enrollment_url),
json={
"course_details": {"course_id": course_id},
"is_active": False,
}
json=params
)
resp.raise_for_status()
return Enrollment(resp.json())
8 changes: 6 additions & 2 deletions edx_api/enrollments/init_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,14 +172,18 @@ def test_deactivate_enrollment(self, request_mock):
"""
request_mock.post(self.enrollment_url, json=self.enrollments_json[0])
course_id = 'course_id'
returned_enrollment = self.enrollment_client.deactivate_enrollment(course_id=course_id)
user = 'user'
returned_enrollment = self.enrollment_client.deactivate_enrollment(
course_id=course_id, username=user
)
self.assertDictEqual(
request_mock.last_request.json(),
{
'course_details': {
'course_id': course_id
},
'is_active': False
'is_active': False,
'user': user
}
)
assert returned_enrollment.json == self.enrollments_json[0]
13 changes: 6 additions & 7 deletions edx_api/integration_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@


ACCESS_TOKEN = os.getenv('ACCESS_TOKEN')
API_KEY = os.getenv('API_KEY')
BASE_URL = os.getenv('BASE_URL', 'http://localhost:18000/')
ENROLLMENT_CREATION_COURSE_ID = os.getenv('ENROLLMENT_CREATION_COURSE_ID')

Expand Down Expand Up @@ -111,7 +110,7 @@ def test_course_structure():
Pull down the course structure and validate it has the correct entries.
This test assumes that the used user can access the course.
"""
api = EdxApi({'access_token': ACCESS_TOKEN, 'api_key': API_KEY}, base_url=BASE_URL)
api = EdxApi({'access_token': ACCESS_TOKEN}, base_url=BASE_URL)
structure = api.course_structure.course_blocks('course-v1:edX+DemoX+Demo_Course', 'staff')

titles = [
Expand All @@ -135,7 +134,7 @@ def test_enrollments():
Enrolls the user in a course and then pulls down the enrollments for the user.
This assumes that the course in the edX instance is available for enrollment.
"""
api = EdxApi({'access_token': ACCESS_TOKEN, 'api_key': API_KEY}, base_url=BASE_URL)
api = EdxApi({'access_token': ACCESS_TOKEN}, base_url=BASE_URL)

# the enrollment will be done manually until
# a client to enroll the student is implemented
Expand All @@ -160,7 +159,7 @@ def test_create_verified_enrollment():
"""
Integration test to enroll the user in a course with `verified` mode
"""
api = EdxApi({'access_token': ACCESS_TOKEN, 'api_key': API_KEY}, base_url=BASE_URL)
api = EdxApi({'access_token': ACCESS_TOKEN}, base_url=BASE_URL)
enrollment = api.enrollments.create_student_enrollment(
course_id=ENROLLMENT_CREATION_COURSE_ID,
mode=ENROLLMENT_MODE_VERIFIED,
Expand All @@ -175,7 +174,7 @@ def test_create_audit_enrollment():
"""
Integration test to enroll the user in a course with `audit` mode
"""
api = EdxApi({'access_token': ACCESS_TOKEN, 'api_key': API_KEY}, base_url=BASE_URL)
api = EdxApi({'access_token': ACCESS_TOKEN}, base_url=BASE_URL)
enrollment = api.enrollments.create_student_enrollment(
course_id=ENROLLMENT_CREATION_COURSE_ID,
mode=ENROLLMENT_MODE_AUDIT,
Expand All @@ -190,7 +189,7 @@ def test_deactivate_enrollment():
"""
Integration test to enroll then deactivate a user in a course
"""
api = EdxApi({'access_token': ACCESS_TOKEN, 'api_key': API_KEY}, base_url=BASE_URL)
api = EdxApi({'access_token': ACCESS_TOKEN}, base_url=BASE_URL)
api.enrollments.create_student_enrollment(
course_id=ENROLLMENT_CREATION_COURSE_ID,
mode=ENROLLMENT_MODE_AUDIT
Expand All @@ -207,7 +206,7 @@ def test_create_enrollment_timeout():
"""
Asserts request timeout error on enrollment api
"""
api = EdxApi({'access_token': ACCESS_TOKEN, 'api_key': API_KEY}, base_url=BASE_URL)
api = EdxApi({'access_token': ACCESS_TOKEN}, base_url=BASE_URL)
enrollments = api.enrollments
with patch.object(enrollments.requester, 'post', autospec=True) as post:
post.side_effect = mocked_timeout
Expand Down
Loading