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

Remove setup_rsession #149

Open
ryanlovett opened this issue Mar 27, 2024 · 1 comment
Open

Remove setup_rsession #149

ryanlovett opened this issue Mar 27, 2024 · 1 comment

Comments

@ryanlovett
Copy link
Collaborator

Proposed change

This extension has two setup functions, setup_rserver and setup_rsession, although only setup_rserver is actually used. Those functions map to RStudio's rserver and rsession executables. rserver is the main executable of RStudio Server and manages user sessions and authentication, while rsession is the executable for individual R sessions. Originally this extension launched rsession because the multi-user capabilities were not needed, however at some point we pivoted to rserver since that seemed to be more reliable for us. I kept the setup_rsession code around in case we ever decided we wanted or needed to go back.

However, keeping the unused function around means that some things (like _get_timeout) are duplicated.

Alternative options

Alternatively, we can move the duplicate parts (_get_timeout) into a larger scope.

Who would use this feature?

Contributors won't have to know where there's an unused function.

(Optional): Suggest a solution

We could also revisit setup_rsession and see if it is still viable, or perhaps the better choice? Perhaps it avoids the whole auth redirect issue that pops up under some circumstances.

@yuvipanda
Copy link
Contributor

yes let's get rid of it!

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

No branches or pull requests

2 participants