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.

Sign up for GitHub

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

@uppy/companion: allow customizing express session prefix #4249

Merged
merged 7 commits into from
Jan 26, 2023
Merged

@uppy/companion: allow customizing express session prefix #4249

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
43a14bd
allow customizing express session prefix
mifi Dec 16, 2022
7746634
rename
mifi Dec 16, 2022
20ef557
document COMPANION_REDIS_EXPRESS_SESSION_PREFIX
mifi Dec 22, 2022
ba96803
change todo
mifi Dec 22, 2022
9952946
Update companion.md
mifi Dec 22, 2022
fea8cc3
Update companion.md
mifi Dec 22, 2022
49ac5eb
Merge branch 'main' into custom-express-session-prefix
Murderlon Jan 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/@uppy/companion/src/standalone/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ module.exports = function server (inputCompanionOptions = {}) {
if (companionOptions.redisUrl) {
const RedisStore = connectRedis(session)
const redisClient = redis.client(companionOptions)
sessionOptions.store = new RedisStore({ client: redisClient })
// todo next major: change default prefix to something like "companion:" and possibly remove this option
sessionOptions.store = new RedisStore({ client: redisClient, prefix: process.env.COMPANION_REDIS_EXPRESS_SESSION_PREFIX || 'sess:' })
Copy link
Member

Choose a reason for hiding this comment

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

What about making the default prefix include companion? Other people running companion may also get confused what these sess: things are in their redis without such clues?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would also prefer that, thrn we might not even need an option. But I think that would be a breaking change and not sure if we have a major of companion coming up

Copy link
Member

Choose a reason for hiding this comment

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

We have a place for storing Major suggestions right? then I propos we keep this now, but add the reminder there for when the next major does come up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we usually store them as TODOs in the code (there is already a todo there to change the default)

}

if (process.env.COMPANION_COOKIE_DOMAIN) {
Expand Down
9 changes: 8 additions & 1 deletion website/src/docs/companion.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,15 @@ export COMPANION_PERIODIC_PING_URLS="https://example.com/ping1,https://example.c
export COMPANION_PERIODIC_PING_INTERVAL=60000
# corresponds to the periodicPingStaticPayload option (JSON string)
export COMPANION_PERIODIC_PING_STATIC_JSON_PAYLOAD="{\"static\":\"data\"}"

# Set a custom prefix for redis keys created by [connect-redis](https://github.com/tj/connect-redis). Defaults to `sess:`. Sessions are used for storing authentication state and for allowing thumbnails to be loaded by the browser via Companion. You might want to change this because if you run a redis with many different apps in the same redis server, it's hard to know where `sess:` comes from and it might collide with other apps. **Note:** in the future ,we plan and changing the default to `companion:` and possibly remove this option.
export COMPANION_REDIS_EXPRESS_SESSION_PREFIX="sess:"
```

when merged https://github.com/transloadit/uppy/pull/4249
mifi marked this conversation as resolved.
Show resolved Hide resolved

````

See [`.env.example`](https://github.com/transloadit/uppy/blob/main/.env.example) for an example environment configuration file.

### Options
Expand Down Expand Up @@ -353,7 +360,7 @@ const options = {
periodicPingStaticPayload: { static: 'payload' },
corsOrigins: true,
}
```
````

1. **filePath(required)** - Full path to the directory to which provider files will be downloaded temporarily.

Expand Down