- 
                Notifications
    
You must be signed in to change notification settings  - Fork 270
 
DOC-4561 fixes for RDI Helm docs #882
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
| ```bash | ||
| tar -xvf rdi-k8s-<rdi-tag>.tar.gz | ||
| tar -xvf rdi-<rdi-tag>.tar.gz | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like a tab character was used before "tar". I suggest replacing this with spaces. Here's an octal dump of the above snippet:
0000000    1   .           D   e   c   o   m   p   r   e   s   s       t
0000020    h   e       t   a   r       f   i   l   e   :  \n  \n        
0000040            `   `   `   b   a   s   h  \n  \t   t   a   r       -
0000060    x   v   f       r   d   i   -   <   r   d   i   -   t   a   g
0000100    >   .   t   a   r   .   g   z      \n                   `   `
0000120    `                                                            
0000121
In VS Code, the tar line does not line up with the indent of the line above.
Doesn't look like this affects the display, so consider it a quality of life change. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| Note that if you want to use | ||
| [Redis Insight]({{< relref "/develop/tools/insight/rdi-connector" >}}) | ||
| to connect to your RDI deployment and you are using TLS, make sure you | ||
| uncomment the `RDI_API_AUTH_ENABLED` line in the default `values.yaml`: | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this setup is needed. TLS ingress is needed though if you're connecting with a RedisInsight outside the k8s cluster
| 
           In   | 
    
| 
           We should make it clear that   | 
    
| 
           There is no mention about how to get   | 
    
| 
           We should make it clear which values need to be set in   | 
    
DOC-4561
Let me know if this needs more emphasis or explanation, and if you have a better value for
JWT_SECRET_KEYthanyourKey, I'll add it in.