Update redis to support tls redis (rediss:)

[mikekwright]

There are two small changes in this PR.

1. To correctly support user being supplied in the redis url for authentication.
2. Enable rediss: as the means to support TLS for redis connections.

If there is a more concise way to be more DRY between the normal and tls redis adapters please let me know.

[tpope]

2. Enable rediss: as the means to support TLS for redis connections.

Is this a standard and/or popular convention, or is it something you invented? Most adapters use a query parameter for this. If we're inventing something, I'd rather invent one of those.

If there is a more concise way to be more DRY between the normal and tls redis adapters please let me know.

There's a couple of ways to alias adapter names.

[mikekwright]

Is this a standard and/or popular convention, or is it something you invented? Most adapters use a query parameter for this. If we're inventing something, I'd rather invent one of those.

This is a great question, this appears to be the common way of supporting TLS as I see this used in a few of the official redis clients including: go-redis, ioredis and redis-py.

There's a couple of ways to alias adapter names.

Great, I will work on an implementation that uses this same pattern and update the PR accordingly. Any other requested changes?

[tpope]

Is this a standard and/or popular convention, or is it something you invented? Most adapters use a query parameter for this. If we're inventing something, I'd rather invent one of those.

This is a great question, this appears to be the common way of supporting TLS as I see this used in a few of the official redis clients including: go-redis, ioredis and redis-py.

Okay, good enough.

There's a couple of ways to alias adapter names.

Great, I will work on an implementation that uses this same pattern and update the PR accordingly. Any other requested changes?

Needed to refresh my memory on how these work. The g:db_adapters form is strictly for alternate names for the same thing. The alias will be replaced with the canonical name during normalization.

The second form, used by redis+srv://, is what you want.

[mikekwright]

Thanks for the guidance, I updated the PR, please let me know if this is more inline with what you would like to see.

[padulkemid]

i was looking for this and got to change it like this in the adapter:

function! db#adapter#redis#interactive(url) abort
  let params = db#url#parse(a:url)
  return ['redis-cli'] + 
    \ (params.scheme ==# 'rediss' || get(params.params, 'tls', '') ==# 'true' ? ['--tls'] : []) +
    \ db#url#as_argv(a:url, '-h ', '-p ', '', '', '-a ', '-n ')
endfunction

and waiting until someone have a change about this (well this one just a temp stuff) but it works really well, i think you have the same implementation

[padulkemid]

hi is this gonna be merged soon? @tpope @mikekwright

[mikekwright]

@padulkemid I hope so, I am not seeing any other changes and just waiting for either a requested update or someone with write access to merge.

[padulkemid]

@mikekwright I think mantainer is not available yet (I hope this can be merged soon to accomodate recent redis update so we're not changing the plugin manually)

[tpope]

The problem is it's not sustainable for me to be responsible for database adapters I don't actively use, especially when I can't apt-get install them to easily things out. Going forward I will be encouraging people to create new plugins to house their adapter submissions, but I haven't decided what to do about the ones I'm already responsible for.