Skip to content

Better channel restoration for local dev with content proxying #5046

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

Open
wants to merge 8 commits into
base: unstable
Choose a base branch
from

Conversation

bjester
Copy link
Member

@bjester bjester commented May 6, 2025

Summary

  • Refactors restore_channel management command to be more robust and useful for restoring channels for local development:
    • No longer downloads content by default
    • Properly restores exercises if API token is provided
    • Supports making the channel public and publishing it after restoration
    • Better progress and logging of restoration
  • Moves all python commands from package.json to Makefile
  • Updates dev server commands to operate through make and to handle always running nginx in front of the devserver
  • Updates documentation to simplify instructions and remove duplicate cruft
  • Adds composite and read-only storage pass through for development like non-production server environments
  • Folds presigned URL handling for uploads into storage structure to eliminate duck typing

References

Needs for building on search recommendations

Reviewer guidance

  1. Restoring a channel: python contentcuration/manage.py restore_channel --editor [email protected] --source-url https://studio.learningequality.org --public --publish --token <your_studio_token> <channel_id> (the following is a small channel: d0ef6f71e4fe4e54bb87d7dab5eeaae2)
  2. Start development server make devrun-services then make devrun-server
  3. Log into local studio
  4. Open the restored channel
  5. Observe you can see the PDF and video resources without having downloaded them (proxying through nginx to the cloud)
  6. Observe the channel exercises are properly restored
  7. Observe new 'Download file' feature now does not simply open the file in a new tab

Notes

I simply deleted the existing tests for the channel restoration. Since it's a development only command, the tests usefulness are questionable, but I'm willing to add tests later if it's desired.

@bjester bjester changed the title Easy channel restoration Better channel restoration for local development with content proxying May 6, 2025
@bjester bjester changed the title Better channel restoration for local development with content proxying Better channel restoration for local dev with content proxying May 6, 2025
@bjester bjester marked this pull request as ready for review May 6, 2025 20:16
@bjester bjester requested review from akolson, ozer550 and MisRob May 6, 2025 20:19
@bjester bjester force-pushed the easy-channel-restoration branch from 8de0abb to 586f089 Compare May 6, 2025 23:09
@bjester bjester force-pushed the easy-channel-restoration branch from f7d9fe4 to 16638f6 Compare May 7, 2025 14:54
)
parser.add_argument(
"--source-url",
default="http://localhost:8080",
Copy link
Member

Choose a reason for hiding this comment

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

port 8080 here is the nginx port?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it is. The default functionality before my changes had this command attempt to load from the local instance, so I kept that, but having this be the nginx route means that the logic for fetching the channel's SQLite database can be the same as non-local (production)

action="store_true",
default=False,
help="Whether to download content files",
)
Copy link
Member

Choose a reason for hiding this comment

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

We surely needed this. Thanks for adding it ❤️

@bjester
Copy link
Member Author

bjester commented May 16, 2025

TODO changes:

  • remove docker-compose alias handling from Makefile
  • potentially remove docker compose handling entirely from Makefile

function _on_interrupt() { $(DOCKER_COMPOSE) stop studio-nginx; }; \
trap _on_interrupt SIGINT SIGTERM SIGKILL ERR; \
$(DOCKER_COMPOSE) up -d studio-nginx; \
$(MAKE) -j 2 devrun-django devrun-webpack
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add an echo here to tell the actual URL to access on, to make sure people ignore the logging from the django runserver?

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