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

Add hybrid method for tutorial storage #518

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

elimillera
Copy link

Hybrid Storage

We discovered when we deployed learnr tutorials to a hosted environment that certain parts of the tutorial would take a long time(30-45 seconds) to initialized. This was because we were using the 'filesystem' option when storing user data and the filesystem was a lot slower as the tutorials got larger and more users accessed it at once. This also provides a fallback if the user deletes their cookies or is blocking them.

Our solution was to combine the client and filesystem methods. This has worked very well for us and we're hoping to contribute it back to the package.

Functions in hybrid_storage are a combination of client and filesystem storage with the exception of get_objects. get_objects includes logic to restore the state in the filesystem to the client storage if there are zero or one items in the client storage.
PR task list:

  • Update NEWS
  • Add tests (if possible)
  • Update documentation with devtools::document()

@elimillera elimillera marked this pull request as ready for review April 26, 2021 15:24
@mstackhouse
Copy link

Just to add to Eli's notes a little:

  • The hybrid storage leverages the speed of client method but the persistence of the filesystem storage
  • This allows users to safely switch between browsers or different computers and preserve their progress
  • If users move to a new environment (i.e. browser or different computer), the filesystem is always the preferred source of truth. This is because all progress is committed to both disk and the client, so progress can always be restored from the filesystem.

This is working quite well for us in practice so we're hoping that this can be of benefit to other users of learnr!

@schloerke schloerke requested a review from gadenbuie August 23, 2021 21:19
Copy link
Member

@gadenbuie gadenbuie left a comment

Choose a reason for hiding this comment

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

Thanks @elimillera and @mstackhouse. I think it's a very reasonable feature request and I like the idea of it. I've looked at the PR a couple of times but haven't had the time required to dig into it.

I will share that my initial thoughts are that, as a hybrid storage system, I would really like to see the feature rely more on the other storage functions. I'll admit I haven't tried to implement it, but I'd really like to see a hybrid_storage() function that delegates to filesystem_storage() and client_storage().

I have reservations, therefore, about bringing an entirely new storage system with a unique implementation. In the long term, I think it'd be difficult to maintain and keep the hybrid system in line with the client and filesystem storage methods.

Of course, please do let me know if I'm missing something in my understanding.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

4 participants