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

feat: add db-session store #722

Merged
merged 4 commits into from Feb 18, 2021
Merged

feat: add db-session store #722

merged 4 commits into from Feb 18, 2021

Conversation

chriswk
Copy link
Contributor

@chriswk chriswk commented Feb 12, 2021

No description provided.

@chriswk chriswk marked this pull request as draft February 12, 2021 13:20
@coveralls
Copy link

coveralls commented Feb 12, 2021

Coverage Status

Coverage decreased (-0.02%) to 95.57% when pulling 4486099 on feat-session-db-store into 8bf4214 on master.

@chriswk
Copy link
Contributor Author

chriswk commented Feb 12, 2021

Some concerns regarding how to handle this properly for tests, we mentioned adding a new option as to whether or not we're using a db backed session store. We should also scrap the cookie-session and just use a MemoryStore for express-session. I'll change this.

@ivarconr
Copy link
Member

Will the store go to database everytime, or is it possible to have some local in-mem cache for hot sessions? If yes we need to think about how logout should work to invalidate local cache. If no we need to do some performance testing to make sure we are not overloading the db

@chriswk chriswk marked this pull request as ready for review February 16, 2021 08:32
@chriswk chriswk force-pushed the feat-session-db-store branch 2 times, most recently from 3a62ddc to 7dede72 Compare February 16, 2021 10:10
@chriswk chriswk temporarily deployed to unleash-db-session February 16, 2021 10:40 Inactive
@chriswk chriswk temporarily deployed to unleash-db-session February 16, 2021 14:24 Inactive
@chriswk chriswk temporarily deployed to unleash-db-session February 17, 2021 14:25 Inactive
@@ -81,6 +81,7 @@ function defaultOptions() {
version,
secureHeaders: process.env.SECURE_HEADERS || false,
enableOAS: process.env.ENABLE_OAS || false,
dbSession: process.env.DB_SESSION || true,
Copy link
Member

Choose a reason for hiding this comment

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

could we introduce a session section? { session: { db: true }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, and move maxAge in there as well then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

introduced in a9c198d

@chriswk chriswk temporarily deployed to unleash-db-session February 17, 2021 14:36 Inactive
@chriswk chriswk temporarily deployed to unleash-db-session February 17, 2021 14:55 Inactive
@chriswk chriswk merged commit d017ec7 into master Feb 18, 2021
@chriswk chriswk deleted the feat-session-db-store branch February 18, 2021 08:10
Tymek pushed a commit that referenced this pull request Aug 26, 2022
* feat: setup new screen structure

* refactor: strategyParameter

* feat: add strategy input errors for required fields

* feat: add create strategy to routes

* feat: add EditStrategy component

* feat: edit strategy view and EditStrategy component

* feat: update EditStrategy component

* test: update snapshots

* fix: styles

* test: update snapshots

* refactor: rename StrategyForm and fix ts errors

* test: update snapshots

* fix: remove test route

* fix: update PR based on feedback

* fix: update PR based on feedback

* refactor: restore feature settings (#712)

* refactor: resotre feature settings

* fix: update PR based on feedback

* feat: add feature information in Metadata container

* fix: update PR based on feedback

* fix: update PR based on feedback

Co-authored-by: Fredrik Strand Oseberg <fredrik.no@gmail.com>

* chore(deps): update dependency @types/react-dom to v17.0.13

* refactor: expect existing TS errors (#767)

* refactor: expect existing TS errors

* refactor: fail build on new TS errors

* fix: styles

* refactor: rename StrategyForm and fix ts errors

* fix: update PR based on feedback

* fix: cleaning up

* fix: remove errors and warnings

* fix: remove ts-expect-error and fix errors

* fix: ts errors

* Update src/component/strategies/StrategyView/StrategyView.tsx

* Update src/component/strategies/StrategyView/StrategyView.tsx

Co-authored-by: Fredrik Strand Oseberg <fredrik.no@gmail.com>
Co-authored-by: Renovate Bot <bot@renovateapp.com>
Co-authored-by: olav <mail@olav.io>
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.

None yet

3 participants