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

Redis URL improvement #23

Closed
skorokithakis opened this issue May 5, 2021 · 8 comments
Closed

Redis URL improvement #23

skorokithakis opened this issue May 5, 2021 · 8 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@skorokithakis
Copy link

Right now, the Redis address can just be a hostname. It would be better if Send supported the full URL (redis://username:password@host:port/dbnum) so we could point it to already-existing Redis servers.

@timvisee
Copy link
Owner

timvisee commented May 5, 2021

You can also set the port and password through a separate variable:

send/server/config.js

Lines 62 to 76 in 385ac59

redis_host: {
format: String,
default: 'localhost',
env: 'REDIS_HOST'
},
redis_port: {
format: Number,
default: 6379,
env: 'REDIS_PORT'
},
redis_password: {
format: String,
default: '',
env: 'REDIS_PASSWORD'
},

But I agree that supporting the more conventional URL would be better.

@timvisee timvisee added enhancement New feature or request good first issue Good for newcomers labels May 5, 2021
@skorokithakis
Copy link
Author

Oh, that's good enough, thanks! I'm trying to set up Send using Compose at the moment, and this seems adequate.

I don't know if you'd be interested to add support, but I wrote a tool for easier deployment of apps such as this one. It's still early, but it's quite useful to me, at least: https://pypi.org/project/docker-harbormaster/

@timvisee
Copy link
Owner

timvisee commented May 5, 2021

Oh, that's good enough, thanks!

Great. I'll leave it open as tracking issue for Redis URL support to be implemented.

I don't know if you'd be interested to add support, but I wrote a tool for easier deployment of apps such as this one. It's still early, but it's quite useful to me, at least: https://pypi.org/project/docker-harbormaster/

Sounds cool. What would 'adding support' mean, having to provide a compose file?

You might find this template useful: https://github.com/timvisee/send-docker-compose

@skorokithakis
Copy link
Author

Yes, it's just providing a compose file with the {{ HM_DATA_DIR }}/my_data:/some_data_dir volumes defined.

Then you could tell people to just add:

repositories:
  timvisee-send:
    url: https://github.com/timvisee/send.git

to their config and it would be automatically spun up. It's probably too early for me to push it very hard, I was just wondering whether something like this would be interesting to app developers.

@pirate
Copy link

pirate commented May 19, 2021

I added more documentation to the Docker / Docker Compose configs for the REDIS_HOST, REDIS_PORT, and REDIS_PASSWORD options here:

Personally I don't think Redis URL support is worth adding, given these options are already present. But I don't have strong opinions either way as long as separate vars stay supported.

imo a higher priority task is to add a REDIS_DB option to select which Redis DB to use e.g. 0, 1, etc. (ability to specify DB no. would also be required to support Redis URLs anyway).

@timvisee
Copy link
Owner

I wonder what the more conventional approach would be?

I've mostly seen separate variables, but maybe Redis URLs are used elsewhere. I'd be happy with both.

@skorokithakis
Copy link
Author

Separate variables are good too, as long as I can specify all of them (host/port/username/pw/db).

@timvisee
Copy link
Owner

I've now added all desired variables: REDIS_{HOST,PORT,USER,PASSWORD,DB}.

I've released this as part of v3.4.11.

It should be available when this pipeline succeeds: https://gitlab.com/timvisee/send/-/pipelines/305718044

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants