Skip to content

feat(SessionPool): allow passing forceCloud down to the KV store#1186

Merged
B4nan merged 3 commits intomasterfrom
feat/allow-force-cloud-for-session-pool
Oct 4, 2021
Merged

feat(SessionPool): allow passing forceCloud down to the KV store#1186
B4nan merged 3 commits intomasterfrom
feat/allow-force-cloud-for-session-pool

Conversation

@vladfrangu
Copy link
Copy Markdown
Member

Closes #752

Might need to change the documentation string, right now it's a 1:1 copy from KV

@vladfrangu vladfrangu requested review from B4nan and pocesar September 21, 2021 11:47
Comment thread src/session_pool/session_pool.js Outdated
Co-authored-by: Paulo Cesar <461084+pocesar@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@pocesar pocesar left a comment

Choose a reason for hiding this comment

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

just a nitpick on docs, otherwise LGTM. it should be advised to use persistStateKeyValueStoreId when using forceCloud: true, since the KV will be an unnamed one and might be confusing for users

@vladfrangu
Copy link
Copy Markdown
Member Author

Would you want me to add that just in documentation, or in code too?

@pocesar
Copy link
Copy Markdown
Contributor

pocesar commented Sep 21, 2021

both a warning (maybe even display the key value store ID created in the warning) and a documentation is good, since people won't know where the sessions went

Copy link
Copy Markdown
Member

@B4nan B4nan left a comment

Choose a reason for hiding this comment

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

agreed with @pocesar, warning would be nice, otherwise lgtm

@B4nan B4nan merged commit b86a4f2 into master Oct 4, 2021
@B4nan B4nan deleted the feat/allow-force-cloud-for-session-pool branch October 4, 2021 09:22
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.

Provide forceCloud to sessionPool

3 participants