feat!: assume & remove BLOCKSTORE_USE_BLOCKSTORE_APP_API (#33765)
Originally, Blockstore was an independent micro-service, accessed via a REST API. Then, we changed Blockstore so it could be installed as an in-process Django app. To support both modes, there existed a blockstore_api wrapper library in edx-platform, with toggles controlling whether the wrapper called out to the micro-service's REST API versus the Django app's Python API. Now that the micro-service Blockstore implementation is deprecated, though, this wrapper library and toggles are just unnecessary complexity. As a first step towards cleanup, we: * remove several toggles and settings (details below); * remove the blocokstore_api wrapper methods which called the REST API and marshalled them back into Python objects; and * remove all test cases which relied on the Blockstore micro-service (and were skippped in CI). In the future, we will remove the content libraries indexer, clean up the remaining bits of blockstore_api, and flatten out all the Blockstore-related test class hierarchies which are no longer nceessary. BREAKING CHANGE: * These Django settings are removed: * BLOCKSTORE_PUBLIC_URL_ROOT * BLOCKSTORE_API_URL * BLOCKSTORE_API_AUTH_TOKEN * BLOCKSTORE_USE_BLOCKSTORE_APP_API * The blockstore.use_blockstore_app_api Waffle switch is removed. * edx-platform will act as it did when the DJango setting BLOCKSTORE_USE_BLOCKSTORE_APP_API or the Waffle switch blockstore.use_blockstore_app_api were enabled. That is, any running Blockstore micro-service instance will be ignored, and the Blockstore package which is installed into edx-platform will be used instead. Ref: https://github.com/openedx/blockstore/issues/296
This commit is contained in:
Kyle McCormick
@@ -115,7 +115,6 @@ from lms.envs.common import (
|
||||
ENTERPRISE_BACKEND_SERVICE_EDX_OAUTH2_PROVIDER_URL,
|
||||
|
||||
# Blockstore
|
||||
BLOCKSTORE_USE_BLOCKSTORE_APP_API,
|
||||
BUNDLE_ASSET_STORAGE_SETTINGS,
|
||||
|
||||
# Methods to derive settings
|
||||
@@ -2606,8 +2605,7 @@ PROCTORING_BACKENDS = {
|
||||
PROCTORING_SETTINGS = {}
|
||||
|
||||
################## BLOCKSTORE RELATED SETTINGS #########################
|
||||
BLOCKSTORE_PUBLIC_URL_ROOT = 'http://localhost:18250'
|
||||
BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1/'
|
||||
|
||||
# Which of django's caches to use for storing anonymous user state for XBlocks
|
||||
# in the blockstore-based XBlock runtime
|
||||
XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'default'
|
||||
|
||||
@@ -40,8 +40,6 @@ AWS_SES_REGION_ENDPOINT: email.us-east-1.amazonaws.com
|
||||
AWS_SES_REGION_NAME: us-east-1
|
||||
AWS_STORAGE_BUCKET_NAME: SET-ME-PLEASE (ex. bucket-name)
|
||||
BASE_COOKIE_DOMAIN: localhost
|
||||
BLOCKSTORE_API_URL: http://localhost:18250/api/v1
|
||||
BLOCKSTORE_PUBLIC_URL_ROOT: http://localhost:18250
|
||||
BLOCK_STRUCTURES_SETTINGS:
|
||||
COURSE_PUBLISH_TASK_DELAY: 30
|
||||
TASK_DEFAULT_RETRY_DELAY: 30
|
||||
|
||||
@@ -27,8 +27,6 @@ from .common import *
|
||||
|
||||
# import settings from LMS for consistent behavior with CMS
|
||||
from lms.envs.test import ( # pylint: disable=wrong-import-order
|
||||
BLOCKSTORE_USE_BLOCKSTORE_APP_API,
|
||||
BLOCKSTORE_API_URL,
|
||||
COMPREHENSIVE_THEME_DIRS, # unimport:skip
|
||||
DEFAULT_FILE_STORAGE,
|
||||
ECOMMERCE_API_URL,
|
||||
@@ -184,10 +182,6 @@ CACHES = {
|
||||
}
|
||||
|
||||
############################### BLOCKSTORE #####################################
|
||||
# Blockstore tests
|
||||
RUN_BLOCKSTORE_TESTS = os.environ.get('EDXAPP_RUN_BLOCKSTORE_TESTS', 'no').lower() in ('true', 'yes', '1')
|
||||
BLOCKSTORE_API_URL = os.environ.get('EDXAPP_BLOCKSTORE_API_URL', "http://edx.devstack.blockstore-test:18251/api/v1/")
|
||||
BLOCKSTORE_API_AUTH_TOKEN = os.environ.get('EDXAPP_BLOCKSTORE_API_AUTH_TOKEN', 'edxapp-test-key')
|
||||
BUNDLE_ASSET_STORAGE_SETTINGS = dict(
|
||||
STORAGE_CLASS='django.core.files.storage.FileSystemStorage',
|
||||
STORAGE_KWARGS=dict(
|
||||
|
||||
@@ -5104,12 +5104,6 @@ RATE_LIMIT_FOR_VIDEO_METADATA_API = '10/minute'
|
||||
MAILCHIMP_NEW_USER_LIST_ID = ""
|
||||
|
||||
########################## BLOCKSTORE #####################################
|
||||
BLOCKSTORE_PUBLIC_URL_ROOT = 'http://localhost:18250'
|
||||
BLOCKSTORE_API_URL = 'http://localhost:18250/api/v1/'
|
||||
|
||||
# Disable the Blockstore app API by default.
|
||||
# See openedx.core.lib.blockstore_api.config for details.
|
||||
BLOCKSTORE_USE_BLOCKSTORE_APP_API = False
|
||||
|
||||
# .. setting_name: XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE
|
||||
# .. setting_default: default
|
||||
|
||||
@@ -57,8 +57,6 @@ AWS_SES_REGION_ENDPOINT: email.us-east-1.amazonaws.com
|
||||
AWS_SES_REGION_NAME: us-east-1
|
||||
AWS_STORAGE_BUCKET_NAME: SET-ME-PLEASE (ex. bucket-name)
|
||||
BASE_COOKIE_DOMAIN: localhost
|
||||
BLOCKSTORE_API_URL: http://localhost:18250/api/v1
|
||||
BLOCKSTORE_PUBLIC_URL_ROOT: http://localhost:18250
|
||||
BLOCK_STRUCTURES_SETTINGS:
|
||||
COURSE_PUBLISH_TASK_DELAY: 30
|
||||
TASK_DEFAULT_RETRY_DELAY: 30
|
||||
|
||||
@@ -557,11 +557,6 @@ add_plugins(__name__, ProjectType.LMS, SettingsType.TEST)
|
||||
derive_settings(__name__)
|
||||
|
||||
############################### BLOCKSTORE #####################################
|
||||
# Blockstore tests
|
||||
RUN_BLOCKSTORE_TESTS = os.environ.get('EDXAPP_RUN_BLOCKSTORE_TESTS', 'no').lower() in ('true', 'yes', '1')
|
||||
BLOCKSTORE_USE_BLOCKSTORE_APP_API = not RUN_BLOCKSTORE_TESTS
|
||||
BLOCKSTORE_API_URL = os.environ.get('EDXAPP_BLOCKSTORE_API_URL', "http://edx.devstack.blockstore-test:18251/api/v1/")
|
||||
BLOCKSTORE_API_AUTH_TOKEN = os.environ.get('EDXAPP_BLOCKSTORE_API_AUTH_TOKEN', 'edxapp-test-key')
|
||||
XBLOCK_RUNTIME_V2_EPHEMERAL_DATA_CACHE = 'blockstore' # This must be set to a working cache for the tests to pass
|
||||
BUNDLE_ASSET_STORAGE_SETTINGS = dict(
|
||||
STORAGE_CLASS='django.core.files.storage.FileSystemStorage',
|
||||
|
||||
@@ -20,8 +20,6 @@ from openedx.core.djangolib.testing.utils import skip_unless_cms
|
||||
from openedx.core.lib import blockstore_api
|
||||
from openedx.core.lib.blockstore_api.tests.base import (
|
||||
BlockstoreAppTestMixin,
|
||||
requires_blockstore,
|
||||
requires_blockstore_app,
|
||||
)
|
||||
|
||||
# Define the URLs here - don't use reverse() because we want to detect
|
||||
@@ -355,15 +353,6 @@ class _ContentLibrariesRestApiTestMixin:
|
||||
return self._api('get', url, None, expect_response=200)["handler_url"]
|
||||
|
||||
|
||||
@requires_blockstore
|
||||
class ContentLibrariesRestApiBlockstoreServiceTest(_ContentLibrariesRestApiTestMixin, APITestCase):
|
||||
"""
|
||||
Base class for Blockstore-based Content Libraries test that use the REST API
|
||||
and the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
@requires_blockstore_app
|
||||
class ContentLibrariesRestApiTest(
|
||||
_ContentLibrariesRestApiTestMixin,
|
||||
BlockstoreAppTestMixin,
|
||||
|
||||
@@ -24,7 +24,6 @@ from openedx_events.content_authoring.signals import (
|
||||
)
|
||||
from openedx.core.djangoapps.content_libraries.libraries_index import LibraryBlockIndexer, ContentLibraryIndexer
|
||||
from openedx.core.djangoapps.content_libraries.tests.base import (
|
||||
ContentLibrariesRestApiBlockstoreServiceTest,
|
||||
ContentLibrariesRestApiTest,
|
||||
elasticsearch_test,
|
||||
URL_BLOCK_METADATA_URL,
|
||||
@@ -1232,16 +1231,6 @@ class ContentLibrariesTestMixin:
|
||||
)
|
||||
|
||||
|
||||
@elasticsearch_test
|
||||
class ContentLibrariesBlockstoreServiceTest(
|
||||
ContentLibrariesTestMixin,
|
||||
ContentLibrariesRestApiBlockstoreServiceTest,
|
||||
):
|
||||
"""
|
||||
General tests for Blockstore-based Content Libraries, using the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
@elasticsearch_test
|
||||
class ContentLibrariesTest(
|
||||
ContentLibrariesTestMixin,
|
||||
|
||||
@@ -11,7 +11,6 @@ from search.search_engine_base import SearchEngine
|
||||
|
||||
from openedx.core.djangoapps.content_libraries.libraries_index import ContentLibraryIndexer, LibraryBlockIndexer
|
||||
from openedx.core.djangoapps.content_libraries.tests.base import (
|
||||
ContentLibrariesRestApiBlockstoreServiceTest,
|
||||
ContentLibrariesRestApiTest,
|
||||
elasticsearch_test,
|
||||
)
|
||||
@@ -181,17 +180,6 @@ class ContentLibraryIndexerTestMixin:
|
||||
verify_uncommitted_libraries(library_key, False, False)
|
||||
|
||||
|
||||
@override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True})
|
||||
@elasticsearch_test
|
||||
class ContentLibraryIndexerBlockstoreServiceTest(
|
||||
ContentLibraryIndexerTestMixin,
|
||||
ContentLibrariesRestApiBlockstoreServiceTest,
|
||||
):
|
||||
"""
|
||||
Tests the operation of ContentLibraryIndexer using the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
@override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True})
|
||||
@elasticsearch_test
|
||||
class ContentLibraryIndexerTest(
|
||||
@@ -303,17 +291,6 @@ class LibraryBlockIndexerTestMixin:
|
||||
assert LibraryBlockIndexer.get_items([block['id']]) == []
|
||||
|
||||
|
||||
@override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True})
|
||||
@elasticsearch_test
|
||||
class LibraryBlockIndexerBlockstoreServiceTest(
|
||||
LibraryBlockIndexerTestMixin,
|
||||
ContentLibrariesRestApiBlockstoreServiceTest,
|
||||
):
|
||||
"""
|
||||
Tests the operation of LibraryBlockIndexer using the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
@override_settings(FEATURES={**settings.FEATURES, 'ENABLE_CONTENT_LIBRARY_INDEX': True})
|
||||
@elasticsearch_test
|
||||
class LibraryBlockIndexerTest(
|
||||
|
||||
@@ -6,7 +6,7 @@ from gettext import GNUTranslations
|
||||
|
||||
from completion.test_utils import CompletionWaffleTestMixin
|
||||
from django.db import connections, transaction
|
||||
from django.test import LiveServerTestCase, TestCase
|
||||
from django.test import LiveServerTestCase
|
||||
from django.utils.text import slugify
|
||||
from organizations.models import Organization
|
||||
from rest_framework.test import APIClient
|
||||
@@ -16,8 +16,6 @@ from lms.djangoapps.courseware.model_data import get_score
|
||||
from openedx.core.djangoapps.content_libraries import api as library_api
|
||||
from openedx.core.djangoapps.content_libraries.tests.base import (
|
||||
BlockstoreAppTestMixin,
|
||||
requires_blockstore,
|
||||
requires_blockstore_app,
|
||||
URL_BLOCK_RENDER_VIEW,
|
||||
URL_BLOCK_GET_HANDLER_URL,
|
||||
URL_BLOCK_METADATA_URL,
|
||||
@@ -224,14 +222,6 @@ class ContentLibraryRuntimeTestMixin(ContentLibraryContentTestMixin):
|
||||
assert xblock_api.get_block_display_name(block_saved) == 'New Display Name'
|
||||
|
||||
|
||||
@requires_blockstore
|
||||
class ContentLibraryRuntimeBServiceTest(ContentLibraryRuntimeTestMixin, TestCase):
|
||||
"""
|
||||
Tests XBlock runtime using XBlocks in a content library using the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
@requires_blockstore_app
|
||||
class ContentLibraryRuntimeTest(ContentLibraryRuntimeTestMixin, BlockstoreAppTestMixin, LiveServerTestCase):
|
||||
"""
|
||||
Tests XBlock runtime using XBlocks in a content library using the installed Blockstore app.
|
||||
@@ -545,14 +535,6 @@ class ContentLibraryXBlockUserStateTestMixin(ContentLibraryContentTestMixin):
|
||||
assert 'Submit' not in dummy_public_view.data['content']
|
||||
|
||||
|
||||
@requires_blockstore
|
||||
class ContentLibraryXBlockUserStateBServiceTest(ContentLibraryXBlockUserStateTestMixin, TestCase): # type: ignore[misc]
|
||||
"""
|
||||
Tests XBlock user state for XBlocks in a content library using the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
@requires_blockstore_app
|
||||
class ContentLibraryXBlockUserStateTest( # type: ignore[misc]
|
||||
ContentLibraryXBlockUserStateTestMixin,
|
||||
BlockstoreAppTestMixin,
|
||||
@@ -619,19 +601,6 @@ class ContentLibraryXBlockCompletionTestMixin(ContentLibraryContentTestMixin, Co
|
||||
assert get_block_completion_status() == 1
|
||||
|
||||
|
||||
@requires_blockstore
|
||||
class ContentLibraryXBlockCompletionBServiceTest(
|
||||
ContentLibraryXBlockCompletionTestMixin,
|
||||
CompletionWaffleTestMixin,
|
||||
TestCase,
|
||||
):
|
||||
"""
|
||||
Test that the Blockstore-based XBlocks can track their completion status
|
||||
using the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
@requires_blockstore_app
|
||||
class ContentLibraryXBlockCompletionTest(
|
||||
ContentLibraryXBlockCompletionTestMixin,
|
||||
CompletionWaffleTestMixin,
|
||||
|
||||
@@ -3,7 +3,6 @@ Tests for static asset files in Blockstore-based Content Libraries
|
||||
"""
|
||||
|
||||
from openedx.core.djangoapps.content_libraries.tests.base import (
|
||||
ContentLibrariesRestApiBlockstoreServiceTest,
|
||||
ContentLibrariesRestApiTest,
|
||||
)
|
||||
|
||||
@@ -109,15 +108,6 @@ class ContentLibrariesStaticAssetsTestMixin:
|
||||
check_download()
|
||||
|
||||
|
||||
class ContentLibrariesStaticAssetsBlockstoreServiceTest(
|
||||
ContentLibrariesStaticAssetsTestMixin,
|
||||
ContentLibrariesRestApiBlockstoreServiceTest,
|
||||
):
|
||||
"""
|
||||
Tests for static asset files in Blockstore-based Content Libraries, using the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
class ContentLibrariesStaticAssetsTest(
|
||||
ContentLibrariesStaticAssetsTestMixin,
|
||||
ContentLibrariesRestApiTest,
|
||||
|
||||
@@ -8,7 +8,6 @@ from django.test import TestCase, override_settings
|
||||
from openedx.core.djangoapps.content_libraries.constants import PROBLEM
|
||||
|
||||
from .base import (
|
||||
ContentLibrariesRestApiBlockstoreServiceTest,
|
||||
ContentLibrariesRestApiTest,
|
||||
URL_LIB_LTI_JWKS,
|
||||
skip_unless_cms,
|
||||
@@ -81,17 +80,6 @@ class LibraryBlockLtiUrlViewTestMixin:
|
||||
self._api("get", '/api/libraries/v2/blocks/lb:CL-TEST:libgg:problem:bad-block/lti/', None, expect_response=404)
|
||||
|
||||
|
||||
@override_features(ENABLE_CONTENT_LIBRARIES=True,
|
||||
ENABLE_CONTENT_LIBRARIES_LTI_TOOL=True)
|
||||
class LibraryBlockLtiUrlViewBlockstoreServiceTest(
|
||||
LibraryBlockLtiUrlViewTestMixin,
|
||||
ContentLibrariesRestApiBlockstoreServiceTest,
|
||||
):
|
||||
"""
|
||||
Test generating LTI URL for a block in a library, using the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
@override_features(ENABLE_CONTENT_LIBRARIES=True,
|
||||
ENABLE_CONTENT_LIBRARIES_LTI_TOOL=True)
|
||||
class LibraryBlockLtiUrlViewTest(
|
||||
|
||||
@@ -5,11 +5,6 @@ from unittest.mock import patch
|
||||
|
||||
from django.test import TestCase
|
||||
from openedx.core.djangolib.blockstore_cache import BundleCache
|
||||
from openedx.core.lib.blockstore_api.tests.base import (
|
||||
BlockstoreAppTestMixin,
|
||||
requires_blockstore,
|
||||
requires_blockstore_app,
|
||||
)
|
||||
from openedx.core.lib import blockstore_api as api
|
||||
|
||||
|
||||
@@ -27,7 +22,7 @@ class TestWithBundleMixin:
|
||||
|
||||
|
||||
@patch('openedx.core.djangolib.blockstore_cache.MAX_BLOCKSTORE_CACHE_DELAY', 0)
|
||||
class BundleCacheTestMixin(TestWithBundleMixin):
|
||||
class BundleCacheTestMixin(TestWithBundleMixin, TestCase):
|
||||
"""
|
||||
Tests for BundleCache
|
||||
"""
|
||||
@@ -112,17 +107,3 @@ class BundleCacheClearTest(TestWithBundleMixin, TestCase):
|
||||
# Now "clear" the cache, forcing the check of the new version:
|
||||
cache.clear()
|
||||
assert cache.get(key1) is None
|
||||
|
||||
|
||||
@requires_blockstore
|
||||
class BundleCacheBlockstoreServiceTest(BundleCacheTestMixin, TestCase):
|
||||
"""
|
||||
Tests BundleCache using the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
@requires_blockstore_app
|
||||
class BundleCacheTest(BundleCacheTestMixin, BlockstoreAppTestMixin, TestCase):
|
||||
"""
|
||||
Tests BundleCache using the installed Blockstore app.
|
||||
"""
|
||||
|
||||
@@ -4,6 +4,9 @@ API Client for Blockstore
|
||||
This API does not do any caching; consider using BundleCache or (in
|
||||
openedx.core.djangolib.blockstore_cache) together with these API methods for
|
||||
improved performance.
|
||||
|
||||
TODO: This wrapper is extraneous now that Blockstore-as-a-service isn't supported.
|
||||
This whole directory tree should be removed by https://github.com/openedx/blockstore/issues/296.
|
||||
"""
|
||||
from blockstore.apps.api.data import (
|
||||
BundleFileData,
|
||||
@@ -16,7 +19,7 @@ from blockstore.apps.api.exceptions import (
|
||||
BundleFileNotFound,
|
||||
BundleStorageError,
|
||||
)
|
||||
from .methods import (
|
||||
from blockstore.apps.api.methods import (
|
||||
# Collections:
|
||||
get_collection,
|
||||
create_collection,
|
||||
|
||||
@@ -1,13 +0,0 @@
|
||||
"""
|
||||
Helper method to indicate when the blockstore app API is enabled.
|
||||
"""
|
||||
from django.conf import settings
|
||||
from .waffle import BLOCKSTORE_USE_BLOCKSTORE_APP_API # pylint: disable=invalid-django-waffle-import
|
||||
|
||||
|
||||
def use_blockstore_app():
|
||||
"""
|
||||
Use the Blockstore app API if the settings say to (e.g. in test)
|
||||
or if the waffle switch is enabled.
|
||||
"""
|
||||
return settings.BLOCKSTORE_USE_BLOCKSTORE_APP_API or BLOCKSTORE_USE_BLOCKSTORE_APP_API.is_enabled()
|
||||
@@ -1,20 +0,0 @@
|
||||
"""
|
||||
Toggles for blockstore.
|
||||
"""
|
||||
|
||||
from edx_toggles.toggles import WaffleSwitch
|
||||
|
||||
# .. toggle_name: blockstore.use_blockstore_app_api
|
||||
# .. toggle_implementation: WaffleSwitch
|
||||
# .. toggle_default: False
|
||||
# .. toggle_description: Enable to use the installed blockstore app's Python API directly instead of the
|
||||
# external blockstore service REST API.
|
||||
# The blockstore REST API is used by default.
|
||||
# .. toggle_use_cases: temporary, open_edx
|
||||
# .. toggle_creation_date: 2022-01-13
|
||||
# .. toggle_target_removal_date: None
|
||||
# .. toggle_tickets: TNL-8705, BD-14
|
||||
# .. toggle_warning: This temporary feature toggle does not have a target removal date.
|
||||
BLOCKSTORE_USE_BLOCKSTORE_APP_API = WaffleSwitch(
|
||||
'blockstore.use_blockstore_app_api', __name__
|
||||
)
|
||||
@@ -1,496 +0,0 @@
|
||||
"""
|
||||
API Client methods for working with Blockstore bundles and drafts
|
||||
"""
|
||||
|
||||
import base64
|
||||
from functools import wraps
|
||||
from urllib.parse import urlencode
|
||||
from uuid import UUID
|
||||
|
||||
import dateutil.parser
|
||||
from django.conf import settings
|
||||
from django.core.exceptions import ImproperlyConfigured
|
||||
import requests
|
||||
|
||||
from blockstore.apps.api.data import (
|
||||
BundleData,
|
||||
CollectionData,
|
||||
DraftData,
|
||||
BundleVersionData,
|
||||
BundleFileData,
|
||||
DraftFileData,
|
||||
BundleLinkData,
|
||||
DraftLinkData,
|
||||
Dependency,
|
||||
)
|
||||
from blockstore.apps.api.exceptions import (
|
||||
NotFound,
|
||||
CollectionNotFound,
|
||||
BundleNotFound,
|
||||
DraftNotFound,
|
||||
BundleFileNotFound,
|
||||
)
|
||||
import blockstore.apps.api.methods as blockstore_api_methods
|
||||
|
||||
from .config import use_blockstore_app
|
||||
|
||||
|
||||
def toggle_blockstore_api(func):
|
||||
"""
|
||||
Decorator function to toggle usage of the Blockstore service
|
||||
and the in-built Blockstore app dependency.
|
||||
"""
|
||||
@wraps(func)
|
||||
def wrapper(*args, **kwargs):
|
||||
if use_blockstore_app():
|
||||
return getattr(blockstore_api_methods, func.__name__)(*args, **kwargs)
|
||||
return func(*args, **kwargs)
|
||||
return wrapper
|
||||
|
||||
|
||||
def api_url(*path_parts):
|
||||
if not settings.BLOCKSTORE_API_URL or not settings.BLOCKSTORE_API_URL.endswith('/api/v1/'):
|
||||
raise ImproperlyConfigured('BLOCKSTORE_API_URL must be set and should end with /api/v1/')
|
||||
return settings.BLOCKSTORE_API_URL + '/'.join(path_parts)
|
||||
|
||||
|
||||
def api_request(method, url, **kwargs):
|
||||
"""
|
||||
Helper method for making a request to the Blockstore REST API
|
||||
"""
|
||||
if not settings.BLOCKSTORE_API_AUTH_TOKEN:
|
||||
raise ImproperlyConfigured("Cannot use Blockstore unless BLOCKSTORE_API_AUTH_TOKEN is set.")
|
||||
kwargs.setdefault('headers', {})['Authorization'] = f"Token {settings.BLOCKSTORE_API_AUTH_TOKEN}"
|
||||
response = requests.request(method, url, **kwargs)
|
||||
if response.status_code == 404:
|
||||
raise NotFound
|
||||
response.raise_for_status()
|
||||
if response.status_code == 204:
|
||||
return None # No content
|
||||
return response.json()
|
||||
|
||||
|
||||
def _collection_from_response(data):
|
||||
"""
|
||||
Given data about a Collection returned by any blockstore REST API, convert it to
|
||||
a CollectionData instance.
|
||||
"""
|
||||
return CollectionData(uuid=UUID(data['uuid']), title=data['title'])
|
||||
|
||||
|
||||
def _bundle_from_response(data):
|
||||
"""
|
||||
Given data about a Bundle returned by any blockstore REST API, convert it to
|
||||
a BundleData instance.
|
||||
"""
|
||||
return BundleData(
|
||||
uuid=UUID(data['uuid']),
|
||||
title=data['title'],
|
||||
description=data['description'],
|
||||
slug=data['slug'],
|
||||
# drafts: Convert from a dict of URLs to a dict of UUIDs:
|
||||
drafts={draft_name: UUID(url.split('/')[-1]) for (draft_name, url) in data['drafts'].items()},
|
||||
# versions field: take the last one and convert it from URL to an int
|
||||
# i.e.: [..., 'https://blockstore/api/v1/bundle_versions/bundle_uuid,15'] -> 15
|
||||
latest_version=int(data['versions'][-1].split(',')[-1]) if data['versions'] else 0,
|
||||
)
|
||||
|
||||
|
||||
def _bundle_version_from_response(data):
|
||||
"""
|
||||
Given data about a BundleVersion returned by any blockstore REST API, convert it to
|
||||
a BundleVersionData instance.
|
||||
"""
|
||||
return BundleVersionData(
|
||||
bundle_uuid=UUID(data['bundle_uuid']),
|
||||
version=data.get('version', 0),
|
||||
change_description=data['change_description'],
|
||||
created_at=dateutil.parser.parse(data['snapshot']['created_at']),
|
||||
files={
|
||||
path: BundleFileData(path=path, **filedata)
|
||||
for path, filedata in data['snapshot']['files'].items()
|
||||
},
|
||||
links={
|
||||
name: BundleLinkData(
|
||||
name=name,
|
||||
direct=Dependency(**link["direct"]),
|
||||
indirect=[Dependency(**ind) for ind in link["indirect"]],
|
||||
)
|
||||
for name, link in data['snapshot']['links'].items()
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _draft_from_response(data):
|
||||
"""
|
||||
Given data about a Draft returned by any blockstore REST API, convert it to
|
||||
a DraftData instance.
|
||||
"""
|
||||
return DraftData(
|
||||
uuid=UUID(data['uuid']),
|
||||
bundle_uuid=UUID(data['bundle_uuid']),
|
||||
name=data['name'],
|
||||
created_at=dateutil.parser.parse(data['staged_draft']['created_at']),
|
||||
updated_at=dateutil.parser.parse(data['staged_draft']['updated_at']),
|
||||
files={
|
||||
path: DraftFileData(path=path, **file)
|
||||
for path, file in data['staged_draft']['files'].items()
|
||||
},
|
||||
links={
|
||||
name: DraftLinkData(
|
||||
name=name,
|
||||
direct=Dependency(**link["direct"]),
|
||||
indirect=[Dependency(**ind) for ind in link["indirect"]],
|
||||
modified=link["modified"],
|
||||
)
|
||||
for name, link in data['staged_draft']['links'].items()
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_collection(collection_uuid):
|
||||
"""
|
||||
Retrieve metadata about the specified collection
|
||||
|
||||
Raises CollectionNotFound if the collection does not exist
|
||||
"""
|
||||
assert isinstance(collection_uuid, UUID)
|
||||
try:
|
||||
data = api_request('get', api_url('collections', str(collection_uuid)))
|
||||
except NotFound:
|
||||
raise CollectionNotFound(f"Collection {collection_uuid} does not exist.") # lint-amnesty, pylint: disable=raise-missing-from
|
||||
return _collection_from_response(data)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def create_collection(title):
|
||||
"""
|
||||
Create a new collection.
|
||||
"""
|
||||
result = api_request('post', api_url('collections'), json={"title": title})
|
||||
return _collection_from_response(result)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def update_collection(collection_uuid, title):
|
||||
"""
|
||||
Update a collection's title
|
||||
"""
|
||||
assert isinstance(collection_uuid, UUID)
|
||||
data = {"title": title}
|
||||
result = api_request('patch', api_url('collections', str(collection_uuid)), json=data)
|
||||
return _collection_from_response(result)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def delete_collection(collection_uuid):
|
||||
"""
|
||||
Delete a collection
|
||||
"""
|
||||
assert isinstance(collection_uuid, UUID)
|
||||
api_request('delete', api_url('collections', str(collection_uuid)))
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_bundles(uuids=None, text_search=None):
|
||||
"""
|
||||
Get the details of all bundles.
|
||||
"""
|
||||
query_params = {}
|
||||
data = {}
|
||||
if uuids:
|
||||
# Potentially we could have a lot of libraries which will lead to 414 error (Request-URI Too Long)
|
||||
# if sending uuids in the query_params. So we have to use the request data instead.
|
||||
data = {'uuid': ','.join(map(str, uuids))}
|
||||
if text_search:
|
||||
query_params['text_search'] = text_search
|
||||
version_url = api_url('bundles') + '?' + urlencode(query_params)
|
||||
response = api_request('get', version_url, json=data)
|
||||
# build bundle from response, convert map object to list and return
|
||||
return [_bundle_from_response(item) for item in response]
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_bundle(bundle_uuid):
|
||||
"""
|
||||
Retrieve metadata about the specified bundle
|
||||
|
||||
Raises BundleNotFound if the bundle does not exist
|
||||
"""
|
||||
assert isinstance(bundle_uuid, UUID)
|
||||
try:
|
||||
data = api_request('get', api_url('bundles', str(bundle_uuid)))
|
||||
except NotFound:
|
||||
raise BundleNotFound(f"Bundle {bundle_uuid} does not exist.") # lint-amnesty, pylint: disable=raise-missing-from
|
||||
return _bundle_from_response(data)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def create_bundle(collection_uuid, slug, title="New Bundle", description=""):
|
||||
"""
|
||||
Create a new bundle.
|
||||
|
||||
Note that description is currently required.
|
||||
"""
|
||||
result = api_request('post', api_url('bundles'), json={
|
||||
"collection_uuid": str(collection_uuid),
|
||||
"slug": slug,
|
||||
"title": title,
|
||||
"description": description,
|
||||
})
|
||||
return _bundle_from_response(result)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def update_bundle(bundle_uuid, **fields):
|
||||
"""
|
||||
Update a bundle's title, description, slug, or collection.
|
||||
"""
|
||||
assert isinstance(bundle_uuid, UUID)
|
||||
data = {}
|
||||
# Most validation will be done by Blockstore, so we don't worry too much about data validation
|
||||
for str_field in ("title", "description", "slug"):
|
||||
if str_field in fields:
|
||||
data[str_field] = fields.pop(str_field)
|
||||
if "collection_uuid" in fields:
|
||||
data["collection_uuid"] = str(fields.pop("collection_uuid"))
|
||||
if fields:
|
||||
raise ValueError(f"Unexpected extra fields passed "
|
||||
f"to update_bundle: {fields.keys()}")
|
||||
result = api_request('patch', api_url('bundles', str(bundle_uuid)), json=data)
|
||||
return _bundle_from_response(result)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def delete_bundle(bundle_uuid):
|
||||
"""
|
||||
Delete a bundle
|
||||
"""
|
||||
assert isinstance(bundle_uuid, UUID)
|
||||
api_request('delete', api_url('bundles', str(bundle_uuid)))
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_draft(draft_uuid):
|
||||
"""
|
||||
Retrieve metadata about the specified draft.
|
||||
If you don't know the draft's UUID, look it up using get_bundle()
|
||||
"""
|
||||
assert isinstance(draft_uuid, UUID)
|
||||
try:
|
||||
data = api_request('get', api_url('drafts', str(draft_uuid)))
|
||||
except NotFound:
|
||||
raise DraftNotFound(f"Draft does not exist: {draft_uuid}") # lint-amnesty, pylint: disable=raise-missing-from
|
||||
return _draft_from_response(data)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_or_create_bundle_draft(bundle_uuid, draft_name):
|
||||
"""
|
||||
Retrieve metadata about the specified draft.
|
||||
"""
|
||||
bundle = get_bundle(bundle_uuid)
|
||||
try:
|
||||
return get_draft(bundle.drafts[draft_name]) # pylint: disable=unsubscriptable-object
|
||||
except KeyError:
|
||||
# The draft doesn't exist yet, so create it:
|
||||
response = api_request('post', api_url('drafts'), json={
|
||||
"bundle_uuid": str(bundle_uuid),
|
||||
"name": draft_name,
|
||||
})
|
||||
# The result of creating a draft doesn't include all the fields we want, so retrieve it now:
|
||||
return get_draft(UUID(response["uuid"]))
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def commit_draft(draft_uuid):
|
||||
"""
|
||||
Commit all of the pending changes in the draft, creating a new version of
|
||||
the associated bundle.
|
||||
|
||||
Does not return any value.
|
||||
"""
|
||||
api_request('post', api_url('drafts', str(draft_uuid), 'commit'))
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def delete_draft(draft_uuid):
|
||||
"""
|
||||
Delete the specified draft, removing any staged changes/files/deletes.
|
||||
|
||||
Does not return any value.
|
||||
"""
|
||||
api_request('delete', api_url('drafts', str(draft_uuid)))
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_bundle_version(bundle_uuid, version_number):
|
||||
"""
|
||||
Get the details of the specified bundle version
|
||||
"""
|
||||
if version_number == 0:
|
||||
return None
|
||||
version_url = api_url('bundle_versions', str(bundle_uuid) + ',' + str(version_number))
|
||||
return _bundle_version_from_response(api_request('get', version_url))
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_bundle_version_files(bundle_uuid, version_number):
|
||||
"""
|
||||
Get a list of the files in the specified bundle version
|
||||
"""
|
||||
if version_number == 0:
|
||||
return []
|
||||
version_info = get_bundle_version(bundle_uuid, version_number)
|
||||
return list(version_info.files.values())
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_bundle_version_links(bundle_uuid, version_number):
|
||||
"""
|
||||
Get a dictionary of the links in the specified bundle version
|
||||
"""
|
||||
if version_number == 0:
|
||||
return {}
|
||||
version_info = get_bundle_version(bundle_uuid, version_number)
|
||||
return version_info.links
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_bundle_files_dict(bundle_uuid, use_draft=None):
|
||||
"""
|
||||
Get a dict of all the files in the specified bundle.
|
||||
|
||||
Returns a dict where the keys are the paths (strings) and the values are
|
||||
BundleFileData or DraftFileData tuples.
|
||||
"""
|
||||
bundle = get_bundle(bundle_uuid)
|
||||
if use_draft and use_draft in bundle.drafts: # pylint: disable=unsupported-membership-test
|
||||
draft_uuid = bundle.drafts[use_draft] # pylint: disable=unsubscriptable-object
|
||||
return get_draft(draft_uuid).files
|
||||
elif not bundle.latest_version:
|
||||
# This bundle has no versions so definitely does not contain any files
|
||||
return {}
|
||||
else:
|
||||
return {file_meta.path: file_meta for file_meta in get_bundle_version_files(bundle_uuid, bundle.latest_version)}
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_bundle_files(bundle_uuid, use_draft=None):
|
||||
"""
|
||||
Get an iterator over all the files in the specified bundle or draft.
|
||||
"""
|
||||
return get_bundle_files_dict(bundle_uuid, use_draft).values()
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_bundle_links(bundle_uuid, use_draft=None):
|
||||
"""
|
||||
Get a dict of all the links in the specified bundle.
|
||||
|
||||
Returns a dict where the keys are the link names (strings) and the values
|
||||
are BundleLinkData or DraftLinkData tuples.
|
||||
"""
|
||||
bundle = get_bundle(bundle_uuid)
|
||||
if use_draft and use_draft in bundle.drafts: # pylint: disable=unsupported-membership-test
|
||||
draft_uuid = bundle.drafts[use_draft] # pylint: disable=unsubscriptable-object
|
||||
return get_draft(draft_uuid).links
|
||||
elif not bundle.latest_version:
|
||||
# This bundle has no versions so definitely does not contain any links
|
||||
return {}
|
||||
else:
|
||||
return get_bundle_version_links(bundle_uuid, bundle.latest_version)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_bundle_file_metadata(bundle_uuid, path, use_draft=None):
|
||||
"""
|
||||
Get the metadata of the specified file.
|
||||
"""
|
||||
assert isinstance(bundle_uuid, UUID)
|
||||
files_dict = get_bundle_files_dict(bundle_uuid, use_draft=use_draft)
|
||||
try:
|
||||
return files_dict[path]
|
||||
except KeyError:
|
||||
raise BundleFileNotFound( # lint-amnesty, pylint: disable=raise-missing-from
|
||||
f"Bundle {bundle_uuid} (draft: {use_draft}) does not contain a file {path}"
|
||||
)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def get_bundle_file_data(bundle_uuid, path, use_draft=None):
|
||||
"""
|
||||
Read all the data in the given bundle file and return it as a
|
||||
binary string.
|
||||
|
||||
Do not use this for large files!
|
||||
"""
|
||||
metadata = get_bundle_file_metadata(bundle_uuid, path, use_draft)
|
||||
with requests.get(metadata.url, stream=True) as r:
|
||||
return r.content
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def write_draft_file(draft_uuid, path, contents):
|
||||
"""
|
||||
Create or overwrite the file at 'path' in the specified draft with the given
|
||||
contents. To delete a file, pass contents=None.
|
||||
|
||||
If you don't know the draft's UUID, look it up using
|
||||
get_or_create_bundle_draft()
|
||||
|
||||
Does not return anything.
|
||||
"""
|
||||
api_request('patch', api_url('drafts', str(draft_uuid)), json={
|
||||
'files': {
|
||||
path: _encode_str_for_draft(contents) if contents is not None else None,
|
||||
},
|
||||
})
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def set_draft_link(draft_uuid, link_name, bundle_uuid, version):
|
||||
"""
|
||||
Create or replace the link with the given name in the specified draft so
|
||||
that it points to the specified bundle version. To delete a link, pass
|
||||
bundle_uuid=None, version=None.
|
||||
|
||||
If you don't know the draft's UUID, look it up using
|
||||
get_or_create_bundle_draft()
|
||||
|
||||
Does not return anything.
|
||||
"""
|
||||
api_request('patch', api_url('drafts', str(draft_uuid)), json={
|
||||
'links': {
|
||||
link_name: {"bundle_uuid": str(bundle_uuid), "version": version} if bundle_uuid is not None else None,
|
||||
},
|
||||
})
|
||||
|
||||
|
||||
def _encode_str_for_draft(input_str):
|
||||
"""
|
||||
Given a string, return UTF-8 representation that is then base64 encoded.
|
||||
"""
|
||||
if isinstance(input_str, str):
|
||||
binary = input_str.encode('utf8')
|
||||
else:
|
||||
binary = input_str
|
||||
return base64.b64encode(binary)
|
||||
|
||||
|
||||
@toggle_blockstore_api
|
||||
def force_browser_url(blockstore_file_url):
|
||||
"""
|
||||
Ensure that the given devstack URL is a URL accessible from the end user's browser.
|
||||
"""
|
||||
# Hack: on some devstacks, we must necessarily use different URLs for
|
||||
# accessing Blockstore file data from within and outside of docker
|
||||
# containers, but Blockstore has no way of knowing which case any particular
|
||||
# request is for. So it always returns a URL suitable for use from within
|
||||
# the container. Only this edxapp can transform the URL at the last second,
|
||||
# knowing that in this case it's going to the user's browser and not being
|
||||
# read by edxapp.
|
||||
# In production, the same S3 URLs get used for internal and external access
|
||||
# so this hack is not necessary.
|
||||
return blockstore_file_url.replace('http://edx.devstack.blockstore:', 'http://localhost:')
|
||||
@@ -1,17 +1,11 @@
|
||||
"""
|
||||
Common code for tests that work with Blockstore
|
||||
"""
|
||||
from unittest import mock, skipUnless
|
||||
from unittest import mock
|
||||
from urllib.parse import urlparse
|
||||
|
||||
from django.conf import settings
|
||||
from django.test.client import RequestFactory
|
||||
|
||||
# Decorators for tests that require the blockstore service/app
|
||||
requires_blockstore = skipUnless(settings.RUN_BLOCKSTORE_TESTS, "Requires a running Blockstore server")
|
||||
|
||||
requires_blockstore_app = skipUnless(settings.BLOCKSTORE_USE_BLOCKSTORE_APP_API, "Requires blockstore app")
|
||||
|
||||
|
||||
class BlockstoreAppTestMixin:
|
||||
"""
|
||||
|
||||
@@ -10,8 +10,6 @@ from django.test import TestCase
|
||||
from openedx.core.lib import blockstore_api as api
|
||||
from openedx.core.lib.blockstore_api.tests.base import (
|
||||
BlockstoreAppTestMixin,
|
||||
requires_blockstore,
|
||||
requires_blockstore_app,
|
||||
)
|
||||
|
||||
# A fake UUID that won't represent any real bundle/draft/collection:
|
||||
@@ -197,14 +195,6 @@ class BlockstoreApiClientTestMixin:
|
||||
assert not api.get_bundle_links(course_bundle.uuid, use_draft=course_draft.name)
|
||||
|
||||
|
||||
@requires_blockstore
|
||||
class BlockstoreServiceApiClientTest(BlockstoreApiClientTestMixin, TestCase):
|
||||
"""
|
||||
Test the Blockstore API Client, using the standalone Blockstore service.
|
||||
"""
|
||||
|
||||
|
||||
@requires_blockstore_app
|
||||
class BlockstoreAppApiClientTest(BlockstoreApiClientTestMixin, BlockstoreAppTestMixin, TestCase):
|
||||
"""
|
||||
Test the Blockstore API Client, using the installed Blockstore app.
|
||||
|
||||
Reference in New Issue
Block a user