From 39a93766d622dc00f3fe550d254e6fed106cb689 Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Mon, 16 Dec 2024 12:43:43 -0500 Subject: [PATCH 1/9] build!: Remove Dockerfile and image building workflows (#36006) These are not maintained by the Open edX community. https://github.com/openedx/public-engineering/issues/263 --- .github/workflows/docker-publish.yml | 43 ---- .github/workflows/publish-ci-docker-image.yml | 43 ---- Dockerfile | 200 ------------------ Makefile | 34 +-- 4 files changed, 1 insertion(+), 319 deletions(-) delete mode 100644 .github/workflows/docker-publish.yml delete mode 100644 .github/workflows/publish-ci-docker-image.yml delete mode 100644 Dockerfile diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml deleted file mode 100644 index 6831e3563d81..000000000000 --- a/.github/workflows/docker-publish.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: Push Docker Images - -on: - push: - branches: - - master - -jobs: - # Push image to GitHub Packages. - # See also https://docs.docker.com/docker-hub/builds/ - push: - runs-on: ubuntu-latest - if: github.event_name == 'push' - - strategy: - matrix: - variant: - - "lms_dev" - - "cms_dev" - - "cms" - - "lms" - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@v3 - - - name: Set up QEMU - uses: docker/setup-qemu-action@v3 - - - name: Login to DockerHub - uses: docker/login-action@v3 - with: - username: ${{ secrets.DOCKERHUB_USERNAME }} - password: ${{ secrets.DOCKERHUB_PASSWORD }} - - - name: Build and push lms/cms base docker images - env: - DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_PASSWORD }} - DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }} - run: make docker_tag_build_push_${{matrix.variant}} diff --git a/.github/workflows/publish-ci-docker-image.yml b/.github/workflows/publish-ci-docker-image.yml deleted file mode 100644 index 6a0f3768b7e6..000000000000 --- a/.github/workflows/publish-ci-docker-image.yml +++ /dev/null @@ -1,43 +0,0 @@ -name: Push CI Runner Docker Image - -on: - workflow_dispatch: - schedule: - - cron: "0 1 * * 3" - -jobs: - push: - runs-on: ubuntu-latest - - steps: - - name: Checkout - uses: actions/checkout@v4 - - # This has to happen after checkout in order for gh to work. - - name: "Cancel scheduled job on forks" - if: github.repository != 'openedx/edx-platform' && github.event_name == 'schedule' - env: - GITHUB_TOKEN: "${{ secrets.GITHUB_TOKEN }}" - run: | - gh run cancel "${{ github.run_id }}" - gh run watch "${{ github.run_id }}" - - - name: Configure AWS Credentials - uses: aws-actions/configure-aws-credentials@v4 - with: - aws-access-key-id: ${{ secrets.TOOLS_EDX_ECR_USER_AWS_ACCESS_KEY_ID }} - aws-secret-access-key: ${{ secrets.TOOLS_EDX_ECR_USER_AWS_SECRET_ACCESS_KEY }} - aws-region: us-east-1 - - - name: Log in to ECR - id: login-ecr - uses: aws-actions/amazon-ecr-login@v2 - - - name: Build, tag, and push image to Amazon ECR - env: - ECR_REGISTRY: ${{ steps.login-ecr.outputs.registry }} - ECR_REPOSITORY: actions-runner - IMAGE_TAG: latest - run: | - docker build -t $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG -f scripts/ci-runner.Dockerfile . - docker push $ECR_REGISTRY/$ECR_REPOSITORY:$IMAGE_TAG diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index a67c46738bcc..000000000000 --- a/Dockerfile +++ /dev/null @@ -1,200 +0,0 @@ -FROM ubuntu:focal as minimal-system - -# Warning: This file is experimental. -# -# Short-term goals: -# * Be a suitable replacement for the `edxops/edxapp` image in devstack (in progress). -# * Take advantage of Docker caching layers: aim to put commands in order of -# increasing cache-busting frequency. -# * Related to ^, use no Ansible or Paver. -# Long-term goal: -# * Be a suitable base for production LMS and CMS images (THIS IS NOT YET THE CASE!). - -ARG DEBIAN_FRONTEND=noninteractive -ARG SERVICE_VARIANT -ARG SERVICE_PORT - -# Env vars: paver -# We intentionally don't use paver in this Dockerfile, but Devstack may invoke paver commands -# during provisioning. Enabling NO_PREREQ_INSTALL tells paver not to re-install Python -# requirements for every paver command, potentially saving a lot of developer time. -ARG NO_PREREQ_INSTALL='1' - -# Env vars: locale -ENV LANG='en_US.UTF-8' -ENV LANGUAGE='en_US:en' -ENV LC_ALL='en_US.UTF-8' - -# Env vars: configuration -ENV CONFIG_ROOT='/edx/etc' -ENV LMS_CFG="$CONFIG_ROOT/lms.yml" -ENV CMS_CFG="$CONFIG_ROOT/cms.yml" - -# Env vars: path -ENV VIRTUAL_ENV="/edx/app/edxapp/venvs/edxapp" -ENV PATH="${VIRTUAL_ENV}/bin:${PATH}" -ENV PATH="/edx/app/edxapp/edx-platform/node_modules/.bin:${PATH}" -ENV PATH="/edx/app/edxapp/edx-platform/bin:${PATH}" -ENV PATH="/edx/app/edxapp/nodeenv/bin:${PATH}" - -WORKDIR /edx/app/edxapp/edx-platform - -# Create user before assigning any directory ownership to it. -RUN useradd -m --shell /bin/false app - -# Use debconf to set locales to be generated when the locales apt package is installed later. -RUN echo "locales locales/default_environment_locale select en_US.UTF-8" | debconf-set-selections -RUN echo "locales locales/locales_to_be_generated multiselect en_US.UTF-8 UTF-8" | debconf-set-selections - -# Setting up ppa deadsnakes to get python 3.11 -RUN apt-get update && \ - apt-get install -y software-properties-common && \ - apt-add-repository -y ppa:deadsnakes/ppa - -# Install requirements that are absolutely necessary -RUN apt-get update && \ - apt-get -y dist-upgrade && \ - apt-get -y install --no-install-recommends \ - python3-pip \ - python3.11 \ - # python3-dev: required for building mysqlclient python package - python3.11-dev \ - python3.11-venv \ - libpython3.11 \ - libpython3.11-stdlib \ - libmysqlclient21 \ - # libmysqlclient-dev: required for building mysqlclient python package - libmysqlclient-dev \ - pkg-config \ - libssl1.1 \ - libxmlsec1-openssl \ - # lynx: Required by https://github.com/openedx/edx-platform/blob/b489a4ecb122/openedx/core/lib/html_to_text.py#L16 - lynx \ - ntp \ - git \ - build-essential \ - gettext \ - gfortran \ - graphviz \ - locales \ - swig \ - && \ - apt-get clean all && \ - rm -rf /var/lib/apt/* - -RUN mkdir -p /edx/var/edxapp -RUN mkdir -p /edx/etc -RUN chown app:app /edx/var/edxapp - -# The builder-production stage is a temporary stage that installs required packages and builds the python virtualenv, -# installs nodejs and node_modules. -# The built artifacts from this stage are then copied to the base stage. -FROM minimal-system as builder-production - -RUN apt-get update && \ - apt-get -y install --no-install-recommends \ - curl \ - libssl-dev \ - libffi-dev \ - libfreetype6-dev \ - libgeos-dev \ - libgraphviz-dev \ - libjpeg8-dev \ - liblapack-dev \ - libpng-dev \ - libsqlite3-dev \ - libxml2-dev \ - libxmlsec1-dev \ - libxslt1-dev - -# Setup python virtual environment -# It is already 'activated' because $VIRTUAL_ENV/bin was put on $PATH -RUN python3.11 -m venv "${VIRTUAL_ENV}" - -# Install python requirements -# Requires copying over requirements files, but not entire repository -COPY requirements requirements -RUN pip install -r requirements/pip.txt -RUN pip install -r requirements/edx/base.txt - -# Install node and npm -RUN nodeenv /edx/app/edxapp/nodeenv --node=20.15.1 --prebuilt -RUN npm install -g npm@10.7.x - -# This script is used by an npm post-install hook. -# We copy it into the image now so that it will be available when we run `npm install` in the next step. -# The script itself will copy certain modules into some uber-legacy parts of edx-platform which still use RequireJS. -COPY scripts/copy-node-modules.sh scripts/copy-node-modules.sh - -# Install node modules -COPY package.json package.json -COPY package-lock.json package-lock.json -RUN npm set progress=false && npm ci - -# The builder-development stage is a temporary stage that installs python modules required for development purposes -# The built artifacts from this stage are then copied to the development stage. -FROM builder-production as builder-development - -RUN pip install -r requirements/edx/development.txt - -# base stage -FROM minimal-system as base - -# Copy python virtual environment, nodejs and node_modules -COPY --from=builder-production /edx/app/edxapp/venvs/edxapp /edx/app/edxapp/venvs/edxapp -COPY --from=builder-production /edx/app/edxapp/nodeenv /edx/app/edxapp/nodeenv -COPY --from=builder-production /edx/app/edxapp/edx-platform/node_modules /edx/app/edxapp/edx-platform/node_modules - -# Copy over remaining parts of repository (including all code) -COPY . . - -# Install Python requirements again in order to capture local projects -RUN pip install -e . - -# Setting edx-platform directory as safe for git commands -RUN git config --global --add safe.directory /edx/app/edxapp/edx-platform - -# Production target -FROM base as production - -USER app - -ENV EDX_PLATFORM_SETTINGS='docker-production' -ENV SERVICE_VARIANT="${SERVICE_VARIANT}" -ENV SERVICE_PORT="${SERVICE_PORT}" -ENV DJANGO_SETTINGS_MODULE="${SERVICE_VARIANT}.envs.$EDX_PLATFORM_SETTINGS" -EXPOSE ${SERVICE_PORT} - -CMD gunicorn \ - -c /edx/app/edxapp/edx-platform/${SERVICE_VARIANT}/docker_${SERVICE_VARIANT}_gunicorn.py \ - --name ${SERVICE_VARIANT} \ - --bind=0.0.0.0:${SERVICE_PORT} \ - --max-requests=1000 \ - --access-logfile \ - - ${SERVICE_VARIANT}.wsgi:application - -# Development target -FROM base as development - -RUN apt-get update && \ - apt-get -y install --no-install-recommends \ - # wget is used in Makefile for common_constraints.txt - wget \ - && \ - apt-get clean all && \ - rm -rf /var/lib/apt/* - -COPY --from=builder-development /edx/app/edxapp/venvs/edxapp /edx/app/edxapp/venvs/edxapp - -RUN ln -s "$(pwd)/lms/envs/devstack-experimental.yml" "$LMS_CFG" -RUN ln -s "$(pwd)/cms/envs/devstack-experimental.yml" "$CMS_CFG" -# Temporary compatibility hack while devstack is supporting both the old `edxops/edxapp` image and this image. -# * Add in a dummy ../edxapp_env file -# * devstack sets /edx/etc/studio.yml as CMS_CFG. -RUN ln -s "$(pwd)/cms/envs/devstack-experimental.yml" "/edx/etc/studio.yml" -RUN touch ../edxapp_env - -ENV EDX_PLATFORM_SETTINGS='devstack_docker' -ENV SERVICE_VARIANT="${SERVICE_VARIANT}" -EXPOSE ${SERVICE_PORT} -CMD ./manage.py ${SERVICE_VARIANT} runserver 0.0.0.0:${SERVICE_PORT} diff --git a/Makefile b/Makefile index 62681f6f3711..8297f4b27b4b 100644 --- a/Makefile +++ b/Makefile @@ -1,8 +1,7 @@ # Do things in edx-platform .PHONY: base-requirements check-types clean \ compile-requirements detect_changed_source_translations dev-requirements \ - docker_auth docker_build docker_tag_build_push_lms docker_tag_build_push_lms_dev \ - docker_tag_build_push_cms docker_tag_build_push_cms_dev docs extract_translations \ + docs extract_translations \ guides help lint-imports local-requirements migrate migrate-lms migrate-cms \ pre-requirements pull pull_xblock_translations pull_translations push_translations \ requirements shell swagger \ @@ -67,9 +66,6 @@ pull_translations: clean_translations ## pull translations via atlas detect_changed_source_translations: ## check if translation files are up-to-date i18n_tool changed -pull: ## update the Docker image used by "make shell" - docker pull edxops/edxapp:latest - pre-requirements: ## install Python requirements for running pip-tools pip install -r requirements/pip.txt pip install -r requirements/pip-tools.txt @@ -94,13 +90,6 @@ test-requirements: pre-requirements requirements: dev-requirements ## install development environment requirements -shell: ## launch a bash shell in a Docker container with all edx-platform dependencies installed - docker run -it -e "NO_PYTHON_UNINSTALL=1" -e "PIP_INDEX_URL=https://pypi.python.org/simple" -e TERM \ - -v `pwd`:/edx/app/edxapp/edx-platform:cached \ - -v edxapp_lms_assets:/edx/var/edxapp/staticfiles/ \ - -v edxapp_node_modules:/edx/app/edxapp/edx-platform/node_modules \ - edxops/edxapp:latest /edx/app/edxapp/devstack.sh open - # Order is very important in this list: files must appear after everything they include! REQ_FILES = \ requirements/edx/coverage \ @@ -164,27 +153,6 @@ upgrade-package: ## update just one package to the latest usable release check-types: ## run static type-checking tests mypy -docker_auth: - echo "$$DOCKERHUB_PASSWORD" | docker login -u "$$DOCKERHUB_USERNAME" --password-stdin - -docker_build: docker_auth - DOCKER_BUILDKIT=1 docker build . --build-arg SERVICE_VARIANT=lms --build-arg SERVICE_PORT=8000 --target development -t openedx/lms-dev - DOCKER_BUILDKIT=1 docker build . --build-arg SERVICE_VARIANT=lms --build-arg SERVICE_PORT=8000 --target production -t openedx/lms - DOCKER_BUILDKIT=1 docker build . --build-arg SERVICE_VARIANT=cms --build-arg SERVICE_PORT=8010 --target development -t openedx/cms-dev - DOCKER_BUILDKIT=1 docker build . --build-arg SERVICE_VARIANT=cms --build-arg SERVICE_PORT=8010 --target production -t openedx/cms - -docker_tag_build_push_lms: docker_auth - docker buildx build -t openedx/lms:latest -t openedx/lms:${GITHUB_SHA} --platform linux/amd64,linux/arm64 --build-arg SERVICE_VARIANT=lms --build-arg SERVICE_PORT=8000 --target production --push . - -docker_tag_build_push_lms_dev: docker_auth - docker buildx build -t openedx/lms-dev:latest -t openedx/lms-dev:${GITHUB_SHA} --platform linux/amd64,linux/arm64 --build-arg SERVICE_VARIANT=lms --build-arg SERVICE_PORT=8000 --target development --push . - -docker_tag_build_push_cms: docker_auth - docker buildx build -t openedx/cms:latest -t openedx/cms:${GITHUB_SHA} --platform linux/amd64,linux/arm64 --build-arg SERVICE_VARIANT=cms --build-arg SERVICE_PORT=8010 --target production --push . - -docker_tag_build_push_cms_dev: docker_auth - docker buildx build -t openedx/cms-dev:latest -t openedx/cms-dev:${GITHUB_SHA} --platform linux/amd64,linux/arm64 --build-arg SERVICE_VARIANT=cms --build-arg SERVICE_PORT=8010 --target development --push . - lint-imports: lint-imports From fb56042bdc125cff060afb61ca4bb6d4cc066f47 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Mon, 16 Dec 2024 15:01:40 -0500 Subject: [PATCH 2/9] temp: Add temporary monitoring for large memcache keys (#36034) We'd like to finish the removal of MD4 key hashing, but want to know if cutting over to BLAKE2b will cause too large a cache turnover. Hopefully this monitoring will give us some insight into the rate of large keys. --- common/djangoapps/util/memcache.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/common/djangoapps/util/memcache.py b/common/djangoapps/util/memcache.py index 2f7e6dc623a0..723ca87098d1 100644 --- a/common/djangoapps/util/memcache.py +++ b/common/djangoapps/util/memcache.py @@ -9,6 +9,7 @@ from django.conf import settings from django.utils.encoding import smart_str +from edx_django_utils.monitoring.utils import increment def fasthash(string): @@ -48,9 +49,18 @@ def safe_key(key, key_prefix, version): # Attempt to combine the prefix, version, and key combined = ":".join([key_prefix, version, key]) + # Temporary: Add observability to large-key hashing to help us + # understand the safety of a cutover from md4 to blake2b hashing. + # See https://github.com/edx/edx-arch-experiments/issues/872 + increment('memcache.safe_key.called') + # If the total length is too long for memcache, hash it if len(combined) > 250: combined = fasthash(combined) + # Temporary: See + # https://github.com/edx/edx-arch-experiments/issues/872 and + # previous comment. + increment('memcache.safe_key.hash_large') # Return the result return combined From 9d2f890f1ec52e0cb98f879a3605792ad0771abf Mon Sep 17 00:00:00 2001 From: BrandonHBodine <5282802+BrandonHBodine@users.noreply.github.com> Date: Mon, 16 Dec 2024 22:07:46 +0000 Subject: [PATCH 3/9] feat: Upgrade Python dependency edxval Bumping edxval to pull in new release Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` --- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f95ae8adb2c9..2fd521330acc 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -549,7 +549,7 @@ edx-when==2.5.0 # via # -r requirements/edx/kernel.in # edx-proctoring -edxval==2.6.1 +edxval==2.7.0 # via -r requirements/edx/kernel.in elasticsearch==7.9.1 # via diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 75c533947407..45e90b16dd34 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -855,7 +855,7 @@ edx-when==2.5.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # edx-proctoring -edxval==2.6.1 +edxval==2.7.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index f7031d349784..3c1d06a28b4f 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -635,7 +635,7 @@ edx-when==2.5.0 # via # -r requirements/edx/base.txt # edx-proctoring -edxval==2.6.1 +edxval==2.7.0 # via -r requirements/edx/base.txt elasticsearch==7.9.1 # via diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 57a0dc6341ad..82e57272137a 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -658,7 +658,7 @@ edx-when==2.5.0 # via # -r requirements/edx/base.txt # edx-proctoring -edxval==2.6.1 +edxval==2.7.0 # via -r requirements/edx/base.txt elasticsearch==7.9.1 # via From 53de4065377272f5a8cb5482aee8846b9d36d42e Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Tue, 17 Dec 2024 16:35:22 +0500 Subject: [PATCH 4/9] feat!: upgrade bulk_beta_modify_access to drf ( 30 ) (#35604) * feat!: upgrading api to DRF. --- lms/djangoapps/instructor/tests/test_api.py | 9 ++ lms/djangoapps/instructor/views/api.py | 151 +++++++++--------- lms/djangoapps/instructor/views/api_urls.py | 2 +- lms/djangoapps/instructor/views/serializer.py | 71 ++++++++ 4 files changed, 158 insertions(+), 75 deletions(-) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 4d3e413f5a2a..f1e7322abc6b 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -1990,6 +1990,15 @@ def test_add_notenrolled_username_autoenroll(self): self.add_notenrolled(response, self.notenrolled_student.username) assert CourseEnrollment.is_enrolled(self.notenrolled_student, self.course.id) + def test_add_notenrolled_username_autoenroll_with_multiple_users(self): + url = reverse('bulk_beta_modify_access', kwargs={'course_id': str(self.course.id)}) + identifiers = (f"Lorem@ipsum.dolor, " + f"sit@amet.consectetur\nadipiscing@elit.Aenean\r convallis@at.lacus\r, ut@lacinia.Sed, " + f"{self.notenrolled_student.username}" + ) + response = self.client.post(url, {'identifiers': identifiers, 'action': 'add', 'email_students': False, 'auto_enroll': True}) # lint-amnesty, pylint: disable=line-too-long + assert 6, len(json.loads(response.content.decode())['results']) + @ddt.data('http', 'https') def test_add_notenrolled_with_email(self, protocol): url = reverse('bulk_beta_modify_access', kwargs={'course_id': str(self.course.id)}) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 631500fe3246..43c9c1947c71 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -109,6 +109,7 @@ CertificateSerializer, CertificateStatusesSerializer, ListInstructorTaskInputSerializer, + ModifyAccessSerializer, RoleNameSerializer, SendEmailSerializer, ShowStudentExtensionSerializer, @@ -914,88 +915,91 @@ def students_update_enrollment(request, course_id): # lint-amnesty, pylint: dis return JsonResponse(response_payload) -@require_POST -@ensure_csrf_cookie -@cache_control(no_cache=True, no_store=True, must_revalidate=True) -@require_course_permission(permissions.CAN_BETATEST) -@common_exceptions_400 -@require_post_params( - identifiers="stringified list of emails and/or usernames", - action="add or remove", -) -def bulk_beta_modify_access(request, course_id): +@method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') +class BulkBetaModifyAccess(DeveloperErrorViewMixin, APIView): """ Enroll or unenroll users in beta testing program. - - Query parameters: - - identifiers is string containing a list of emails and/or usernames separated by - anything split_input_list can handle. - - action is one of ['add', 'remove'] """ - course_id = CourseKey.from_string(course_id) - action = request.POST.get('action') - identifiers_raw = request.POST.get('identifiers') - identifiers = _split_input_list(identifiers_raw) - email_students = _get_boolean_param(request, 'email_students') - auto_enroll = _get_boolean_param(request, 'auto_enroll') - results = [] - rolename = 'beta' - course = get_course_by_id(course_id) + permission_classes = (IsAuthenticated, permissions.InstructorPermission) + permission_name = permissions.CAN_BETATEST + serializer_class = ModifyAccessSerializer - email_params = {} - if email_students: - secure = request.is_secure() - email_params = get_email_params(course, auto_enroll=auto_enroll, secure=secure) + @method_decorator(ensure_csrf_cookie) + def post(self, request, course_id): + """ + Query parameters: + - identifiers is string containing a list of emails and/or usernames separated by + anything split_input_list can handle. + - action is one of ['add', 'remove'] + """ + course_id = CourseKey.from_string(course_id) + serializer = self.serializer_class(data=request.data) + if not serializer.is_valid(): + return JsonResponse({'message': serializer.errors}, status=400) - for identifier in identifiers: - try: - error = False - user_does_not_exist = False - user = get_student_from_identifier(identifier) - user_active = user.is_active + action = serializer.validated_data['action'] + identifiers = serializer.validated_data['identifiers'] + email_students = serializer.validated_data['email_students'] + auto_enroll = serializer.validated_data['auto_enroll'] - if action == 'add': - allow_access(course, user, rolename) - elif action == 'remove': - revoke_access(course, user, rolename) + results = [] + rolename = 'beta' + course = get_course_by_id(course_id) + + email_params = {} + if email_students: + secure = request.is_secure() + email_params = get_email_params(course, auto_enroll=auto_enroll, secure=secure) + + for identifier in identifiers: + try: + error = False + user_does_not_exist = False + user = get_student_from_identifier(identifier) + user_active = user.is_active + + if action == 'add': + allow_access(course, user, rolename) + elif action == 'remove': + revoke_access(course, user, rolename) + else: + return HttpResponseBadRequest(strip_tags( + f"Unrecognized action '{action}'" + )) + except User.DoesNotExist: + error = True + user_does_not_exist = True + user_active = None + # catch and log any unexpected exceptions + # so that one error doesn't cause a 500. + except Exception as exc: # pylint: disable=broad-except + log.exception("Error while #{}ing student") + log.exception(exc) + error = True else: - return HttpResponseBadRequest(strip_tags( - f"Unrecognized action '{action}'" - )) - except User.DoesNotExist: - error = True - user_does_not_exist = True - user_active = None - # catch and log any unexpected exceptions - # so that one error doesn't cause a 500. - except Exception as exc: # pylint: disable=broad-except - log.exception("Error while #{}ing student") - log.exception(exc) - error = True - else: - # If no exception thrown, see if we should send an email - if email_students: - send_beta_role_email(action, user, email_params) - # See if we should autoenroll the student - if auto_enroll: - # Check if student is already enrolled - if not is_user_enrolled_in_course(user, course_id): - CourseEnrollment.enroll(user, course_id) + # If no exception thrown, see if we should send an email + if email_students: + send_beta_role_email(action, user, email_params) + # See if we should autoenroll the student + if auto_enroll: + # Check if student is already enrolled + if not is_user_enrolled_in_course(user, course_id): + CourseEnrollment.enroll(user, course_id) - finally: - # Tabulate the action result of this email address - results.append({ - 'identifier': identifier, - 'error': error, # pylint: disable=used-before-assignment - 'userDoesNotExist': user_does_not_exist, # pylint: disable=used-before-assignment - 'is_active': user_active # pylint: disable=used-before-assignment - }) + finally: + # Tabulate the action result of this email address + results.append({ + 'identifier': identifier, + 'error': error, # pylint: disable=used-before-assignment + 'userDoesNotExist': user_does_not_exist, # pylint: disable=used-before-assignment + 'is_active': user_active # pylint: disable=used-before-assignment + }) - response_payload = { - 'action': action, - 'results': results, - } - return JsonResponse(response_payload) + response_payload = { + 'action': action, + 'results': results, + } + return JsonResponse(response_payload) @method_decorator(cache_control(no_cache=True, no_store=True, must_revalidate=True), name='dispatch') @@ -1025,7 +1029,6 @@ def post(self, request, course_id): course = get_course_with_access( request.user, 'instructor', course_id, depth=None ) - serializer_data = AccessSerializer(data=request.data) if not serializer_data.is_valid(): return HttpResponseBadRequest(reason=serializer_data.errors) diff --git a/lms/djangoapps/instructor/views/api_urls.py b/lms/djangoapps/instructor/views/api_urls.py index ea27034d0942..6f67a1a6863c 100644 --- a/lms/djangoapps/instructor/views/api_urls.py +++ b/lms/djangoapps/instructor/views/api_urls.py @@ -25,7 +25,7 @@ path('register_and_enroll_students', api.RegisterAndEnrollStudents.as_view(), name='register_and_enroll_students'), path('list_course_role_members', api.ListCourseRoleMembersView.as_view(), name='list_course_role_members'), path('modify_access', api.ModifyAccess.as_view(), name='modify_access'), - path('bulk_beta_modify_access', api.bulk_beta_modify_access, name='bulk_beta_modify_access'), + path('bulk_beta_modify_access', api.BulkBetaModifyAccess.as_view(), name='bulk_beta_modify_access'), path('get_problem_responses', api.get_problem_responses, name='get_problem_responses'), path('get_issued_certificates/', api.GetIssuedCertificates.as_view(), name='get_issued_certificates'), re_path(r'^get_students_features(?P/csv)?$', api.GetStudentsFeatures.as_view(), name='get_students_features'), diff --git a/lms/djangoapps/instructor/views/serializer.py b/lms/djangoapps/instructor/views/serializer.py index f7fc685f658c..7b20eed32f60 100644 --- a/lms/djangoapps/instructor/views/serializer.py +++ b/lms/djangoapps/instructor/views/serializer.py @@ -1,4 +1,5 @@ """ Instructor apis serializers. """ +import re from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.core.exceptions import ValidationError @@ -232,6 +233,76 @@ def __init__(self, *args, **kwargs): self.fields['due_datetime'].required = False +class ModifyAccessSerializer(serializers.Serializer): + """ + serializers for enroll or un-enroll users in beta testing program. + """ + identifiers = serializers.CharField( + help_text="A comma separated list of emails or usernames.", + required=True + ) + action = serializers.ChoiceField( + choices=["add", "remove"], + help_text="Action to perform: add or remove.", + required=True + ) + + email_students = serializers.BooleanField( + default=False, + help_text="Boolean flag to indicate if students should be emailed." + ) + + auto_enroll = serializers.BooleanField( + default=False, + help_text="Boolean flag to indicate if the user should be auto-enrolled." + ) + + def validate_identifiers(self, value): + """ + Validate the 'identifiers' field which is now a list of strings. + """ + # Iterate over the list of identifiers and validate each one + validated_list = _split_input_list(value) + if not validated_list: + raise serializers.ValidationError("The identifiers list cannot be empty.") + + return validated_list + + def validate_email_students(self, value): + """ + handle string values like 'true' or 'false'. + """ + if isinstance(value, str): + return value.lower() == 'true' + return bool(value) + + def validate_auto_enroll(self, value): + """ + handle string values like 'true' or 'false'. + """ + if isinstance(value, str): + return value.lower() == 'true' + return bool(value) + + +def _split_input_list(str_list): + """ + Separate out individual student email from the comma, or space separated string. + + e.g. + in: "Lorem@ipsum.dolor, sit@amet.consectetur\nadipiscing@elit.Aenean\r convallis@at.lacus\r, ut@lacinia.Sed" + out: ['Lorem@ipsum.dolor', 'sit@amet.consectetur', 'adipiscing@elit.Aenean', 'convallis@at.lacus', 'ut@lacinia.Sed'] + + `str_list` is a string coming from an input text area + returns a list of separated values + """ + new_list = re.split(r'[,\s\n\r]+', str_list) + new_list = [s.strip() for s in new_list] + new_list = [s for s in new_list if s != ''] + + return new_list + + class CertificateStatusesSerializer(serializers.Serializer): """ Serializer for validating and serializing certificate status inputs. From 5bf0b2704f782904be0b1fcfa7355ae486e2ea46 Mon Sep 17 00:00:00 2001 From: Bryann Valderrama <64033729+BryanttV@users.noreply.github.com> Date: Tue, 17 Dec 2024 09:52:56 -0500 Subject: [PATCH 5/9] feat: add schedule queryset request filter integration (#35982) * feat: add schedule queryset request filter integration * chore: remove try-except when running filter * chore: upgrade openedx-filters to v1.12.0 * test: add filter unit tests * test: inherit of ModuleStoreTestCase * fix: add missing attribute in resolver instance --- .../core/djangoapps/schedules/resolvers.py | 5 ++ .../schedules/tests/test_filters.py | 71 +++++++++++++++++++ requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 6 files changed, 80 insertions(+), 4 deletions(-) create mode 100644 openedx/core/djangoapps/schedules/tests/test_filters.py diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index 5f769e6af299..a07f9c5de7f5 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -14,6 +14,7 @@ from edx_ace.recipient import Recipient from edx_ace.recipient_resolver import RecipientResolver from edx_django_utils.monitoring import function_trace, set_custom_attribute +from openedx_filters.learning.filters import ScheduleQuerySetRequested from lms.djangoapps.courseware.utils import verified_upgrade_deadline_link, can_show_verified_upgrade from lms.djangoapps.discussion.notification_prefs.views import UsernameCipher @@ -154,6 +155,10 @@ def get_schedules_with_target_date_by_bin_and_orgs( schedules = self.filter_by_org(schedules) + # .. filter_implemented_name: ScheduleQuerySetRequested + # .. filter_type: org.openedx.learning.schedule.queryset.requested.v1 + schedules = ScheduleQuerySetRequested.run_filter(schedules) + if "read_replica" in settings.DATABASES: schedules = schedules.using("read_replica") diff --git a/openedx/core/djangoapps/schedules/tests/test_filters.py b/openedx/core/djangoapps/schedules/tests/test_filters.py new file mode 100644 index 000000000000..9f7efa050447 --- /dev/null +++ b/openedx/core/djangoapps/schedules/tests/test_filters.py @@ -0,0 +1,71 @@ +""" +Test cases for the Open edX Filters associated with the schedule app. +""" + +import datetime +from unittest.mock import Mock + +from django.db.models.query import QuerySet +from django.test import override_settings +from openedx_filters import PipelineStep + +from openedx.core.djangoapps.schedules.resolvers import BinnedSchedulesBaseResolver +from openedx.core.djangoapps.schedules.tests.test_resolvers import SchedulesResolverTestMixin +from openedx.core.djangolib.testing.utils import skip_unless_lms +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase + + +class TestScheduleQuerySetRequestedPipelineStep(PipelineStep): + """Pipeline step class to test a configured pipeline step""" + + filtered_schedules = Mock(spec=QuerySet, __len__=Mock(return_value=0)) + + def run_filter(self, schedules: QuerySet): # pylint: disable=arguments-differ + """Pipeline step to filter the schedules""" + return { + "schedules": self.filtered_schedules, + } + + +@skip_unless_lms +class ScheduleQuerySetRequestedFiltersTest(SchedulesResolverTestMixin, ModuleStoreTestCase): + """ + Tests for the Open edX Filters associated with the schedule queryset requested. + + The following filters are tested: + - ScheduleQuerySetRequested + """ + + def setUp(self): + super().setUp() + self.resolver = BinnedSchedulesBaseResolver( + async_send_task=Mock(name="async_send_task"), + site=self.site, + target_datetime=datetime.datetime.now(), + day_offset=3, + bin_num=2, + ) + self.resolver.schedule_date_field = "created" + + @override_settings( + OPEN_EDX_FILTERS_CONFIG={ + "org.openedx.learning.schedule.queryset.requested.v1": { + "pipeline": [ + "openedx.core.djangoapps.schedules.tests.test_filters.TestScheduleQuerySetRequestedPipelineStep", + ], + "fail_silently": False, + }, + }, + ) + def test_schedule_with_queryset_requested_filter_enabled(self) -> None: + """Test to verify the schedule queryset was modified by the pipeline step.""" + schedules = self.resolver.get_schedules_with_target_date_by_bin_and_orgs() + + self.assertEqual(TestScheduleQuerySetRequestedPipelineStep.filtered_schedules, schedules) + + @override_settings(OPEN_EDX_FILTERS_CONFIG={}) + def test_schedule_with_queryset_requested_filter_disabled(self) -> None: + """Test to verify the schedule queryset was not modified when the pipeline step is not configured.""" + schedules = self.resolver.get_schedules_with_target_date_by_bin_and_orgs() + + self.assertNotEqual(TestScheduleQuerySetRequestedPipelineStep.filtered_schedules, schedules) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f95ae8adb2c9..ccedfe5a0227 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -836,7 +836,7 @@ openedx-events==9.15.0 # edx-name-affirmation # event-tracking # ora2 -openedx-filters==1.11.0 +openedx-filters==1.12.0 # via # -r requirements/edx/kernel.in # lti-consumer-xblock diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 75c533947407..0191c779d363 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1390,7 +1390,7 @@ openedx-events==9.15.0 # edx-name-affirmation # event-tracking # ora2 -openedx-filters==1.11.0 +openedx-filters==1.12.0 # via # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index f7031d349784..dacd1efa6143 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -1005,7 +1005,7 @@ openedx-events==9.15.0 # edx-name-affirmation # event-tracking # ora2 -openedx-filters==1.11.0 +openedx-filters==1.12.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 57a0dc6341ad..db1ed7889667 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1050,7 +1050,7 @@ openedx-events==9.15.0 # edx-name-affirmation # event-tracking # ora2 -openedx-filters==1.11.0 +openedx-filters==1.12.0 # via # -r requirements/edx/base.txt # lti-consumer-xblock From f0eb0da1bab5e5d769f22853d811312654975195 Mon Sep 17 00:00:00 2001 From: Tim McCormack Date: Tue, 17 Dec 2024 10:35:08 -0500 Subject: [PATCH 6/9] Revert "temp: Add temporary monitoring for large memcache keys (#36034)" (#36040) This reverts commit fb56042bdc125cff060afb61ca4bb6d4cc066f47. We have the data we need. --- common/djangoapps/util/memcache.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/common/djangoapps/util/memcache.py b/common/djangoapps/util/memcache.py index 723ca87098d1..2f7e6dc623a0 100644 --- a/common/djangoapps/util/memcache.py +++ b/common/djangoapps/util/memcache.py @@ -9,7 +9,6 @@ from django.conf import settings from django.utils.encoding import smart_str -from edx_django_utils.monitoring.utils import increment def fasthash(string): @@ -49,18 +48,9 @@ def safe_key(key, key_prefix, version): # Attempt to combine the prefix, version, and key combined = ":".join([key_prefix, version, key]) - # Temporary: Add observability to large-key hashing to help us - # understand the safety of a cutover from md4 to blake2b hashing. - # See https://github.com/edx/edx-arch-experiments/issues/872 - increment('memcache.safe_key.called') - # If the total length is too long for memcache, hash it if len(combined) > 250: combined = fasthash(combined) - # Temporary: See - # https://github.com/edx/edx-arch-experiments/issues/872 and - # previous comment. - increment('memcache.safe_key.hash_large') # Return the result return combined From 882475dd5e81df717100ae3e54f838c326370494 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:02:36 +0000 Subject: [PATCH 7/9] feat: Upgrade Python dependency edx-enterprise (#36041) Bump version of edx-enterprise to v5.5.0. Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: adamstankiewicz <2828721+adamstankiewicz@users.noreply.github.com> --- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index d7608873644e..541197d83242 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -78,7 +78,7 @@ django-storages<1.14.4 # The team that owns this package will manually bump this package rather than having it pulled in automatically. # This is to allow them to better control its deployment and to do it in a process that works better # for them. -edx-enterprise==5.4.2 +edx-enterprise==5.5.0 # Date: 2024-05-09 # This has to be constrained as well because newer versions of edx-i18n-tools need the diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 62a7d37c054a..dfcc8bcc5dd9 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -472,7 +472,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==5.4.2 +edx-enterprise==5.5.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 76307eacdcb2..15f08c1377c0 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -748,7 +748,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==5.4.2 +edx-enterprise==5.5.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index f9e2be39f4c2..d23eba58993d 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -555,7 +555,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==5.4.2 +edx-enterprise==5.5.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 70bed474e154..98178143005d 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -576,7 +576,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==5.4.2 +edx-enterprise==5.5.0 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From 98214a54167444781115c68593d5901c29cd6098 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:22:01 -0500 Subject: [PATCH 8/9] feat: Upgrade Python dependency edx-enterprise (#36042) Commit generated by workflow `openedx/edx-platform/.github/workflows/upgrade-one-python-dependency.yml@refs/heads/master` Co-authored-by: bcitro <67378070+bcitro@users.noreply.github.com> --- requirements/constraints.txt | 2 +- requirements/edx/base.txt | 2 +- requirements/edx/development.txt | 2 +- requirements/edx/doc.txt | 2 +- requirements/edx/testing.txt | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index 541197d83242..24d822f08744 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -78,7 +78,7 @@ django-storages<1.14.4 # The team that owns this package will manually bump this package rather than having it pulled in automatically. # This is to allow them to better control its deployment and to do it in a process that works better # for them. -edx-enterprise==5.5.0 +edx-enterprise==5.5.1 # Date: 2024-05-09 # This has to be constrained as well because newer versions of edx-i18n-tools need the diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index dfcc8bcc5dd9..6f942d00b096 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -472,7 +472,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==5.5.0 +edx-enterprise==5.5.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 15f08c1377c0..97b74c737b6f 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -748,7 +748,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==5.5.0 +edx-enterprise==5.5.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index d23eba58993d..4261f47c2be7 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -555,7 +555,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==5.5.0 +edx-enterprise==5.5.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 98178143005d..dbc2ea7613f5 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -576,7 +576,7 @@ edx-drf-extensions==10.5.0 # edx-when # edxval # openedx-learning -edx-enterprise==5.5.0 +edx-enterprise==5.5.1 # via # -c requirements/edx/../constraints.txt # -r requirements/edx/base.txt From e2a4b9e52411574216137ae4dee08c7a15adae22 Mon Sep 17 00:00:00 2001 From: Ahtisham Shahid Date: Wed, 18 Dec 2024 15:38:05 +0500 Subject: [PATCH 9/9] fix: updated goal reminder email time logger (#36045) --- .../course_goals/management/commands/goal_reminder_email.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py index eea03bd79455..a221edc8455f 100644 --- a/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py +++ b/lms/djangoapps/course_goals/management/commands/goal_reminder_email.py @@ -1,6 +1,7 @@ """ Command to trigger sending reminder emails for learners to achieve their Course Goals """ +import time from datetime import date, datetime, timedelta from eventtracking import tracker import logging @@ -119,9 +120,9 @@ def send_ace_message(goal, session_id): with emulate_http_request(site, user): try: - start_time = datetime.now() + start_time = time.perf_counter() ace.send(msg) - end_time = datetime.now() + end_time = time.perf_counter() log.info(f"Goal Reminder for {user.id} for course {goal.course_key} sent in {end_time - start_time} " f"using {'SES' if is_ses_enabled else 'others'}") except Exception as exc: # pylint: disable=broad-except