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

Use environments for Connect content + user caches #667

Merged
merged 6 commits into from
Nov 1, 2022

Conversation

juliasilge
Copy link
Member

Addresses #635

Instead of removing these troublesome caches like #660 was headed for, this PR instead changes these caches over to use environments (instead of files on disk). This means that:

  • caches will not persist between sessions
  • caches will only get into a broken state if a big change is made with the server while a user is working with pins from R
  • caches can be fixed by restarting R
  • the first time functions are called they will be slow, but subsequent calls will have the same benefits of caching as we have now

Comment on lines 391 to 392
rsconnect_content_cache_env <- rlang::new_environment()
rsconnect_user_cache_env <- rlang::new_environment()
Copy link
Member Author

Choose a reason for hiding this comment

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

I put these here? And also I think it's simplest to still have two of them, one for the pieces of content and one for the users.

Copy link
Member

Choose a reason for hiding this comment

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

I think this is a totally reasonable approach but we're starting to build a bit of a consensus around using the as a sentinel for a mutate package environment. If you wanted to use that, I think you'd do:

the <- rlang::new_environment()
the$content_cache <- rlang::new_environment()
the$user_cache <-  rlang::new_environment()

@juliasilge
Copy link
Member Author

This idea worked out great IMO @hadley!

Comment on lines 391 to 392
rsconnect_content_cache_env <- rlang::new_environment()
rsconnect_user_cache_env <- rlang::new_environment()
Copy link
Member

Choose a reason for hiding this comment

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

I think this is a totally reasonable approach but we're starting to build a bit of a consensus around using the as a sentinel for a mutate package environment. If you wanted to use that, I think you'd do:

the <- rlang::new_environment()
the$content_cache <- rlang::new_environment()
the$user_cache <-  rlang::new_environment()

@machow
Copy link
Collaborator

machow commented Nov 1, 2022

Just a heads up -- AFAICT one side-effect of this change is that if users restart their R sessions, they'll be unable to access cached RSC pin content without an internet connection. The issue is that already accessed content will be in their on-disk cache, but there will be no mapping back to it :/

(it might be worth letting people access pins via guid in the future to work around? see rstudio/pins-python#79)

@juliasilge
Copy link
Member Author

@machow That is not different than if we turned the cache off via an environment variable or eventually removed the cache altogether, correct? Those were the other options we have been talking about.

I like the idea of using the GUID as an internal bit of metadata.

@github-actions
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Nov 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants