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.

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

feat!: assume & remove BLOCKSTORE_USE_BLOCKSTORE_APP_API #33765

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented Nov 21, 2023

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 ncessary.

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: openedx-unsupported/blockstore#296

@kdmccormick kdmccormick force-pushed the kdmccormick/blockstore-service-depr-1 branch from c5850d8 to 424c0c9 Compare December 6, 2023 14:56
@kdmccormick kdmccormick enabled auto-merge (squash) December 6, 2023 14:58
Copy link
Contributor

@connorhaugh connorhaugh left a comment

Choose a reason for hiding this comment

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

Super fast pass, but looks normal to me.

@kdmccormick kdmccormick merged commit 27803f5 into openedx:master Dec 6, 2023
63 checks passed
@kdmccormick kdmccormick deleted the kdmccormick/blockstore-service-depr-1 branch December 6, 2023 15:24
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants