Skip to content

[DPE-9311] Initial user documentation#24

Open
reneradoi wants to merge 7 commits into9/edgefrom
initial-docs
Open

[DPE-9311] Initial user documentation#24
reneradoi wants to merge 7 commits into9/edgefrom
initial-docs

Conversation

@reneradoi
Copy link
Contributor

This PR adds the initial set of user-facing documentation:

  • a README file
  • a tutorial
  • and several How-to guides for:
    • deployment
    • horizontal scaling
    • managing the passwords of internal users

Copy link
Contributor

@izmalk izmalk left a comment

Choose a reason for hiding this comment

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

Nicely done! I've added a few suggestions but mostly formatting. How-to guide can be improved a bit, but nothing critical.

@@ -0,0 +1,68 @@
# How to disable TLS
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit surprised to see a How to disable TLS guide =).
Question: Is there a chance we can just add it as a section to the How to Enable TLS guide? It is quite small. The only reason I can think of to keep it separate is to boost its visibility and discoverability.


To disable the client-to-server communication, run:

```text
Copy link
Contributor

Choose a reason for hiding this comment

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

Question: Are you sure you want to use the text here? It's fine; it just disables syntax highlighting.

Comment on lines +62 to +64
$ valkey-cli -h 10.1.44.126 -p 6379
10.1.44.126:6379> ping
(error) NOAUTH Authentication required.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Let's avoid using prompts in the code blocks. The best practice is to either use the terminal block or separate input from output and not use prompts.

Suggested change
$ valkey-cli -h 10.1.44.126 -p 6379
10.1.44.126:6379> ping
(error) NOAUTH Authentication required.
valkey-cli -h 10.1.44.126 -p 6379
```
You should see an authentication error as the result since the newtork connection was established, but no credentials provided:
```text
ping
(error) NOAUTH Authentication required.
```

charm as an example for all cases.

```{caution}
**[Self-signed certificates](https://en.wikipedia.org/wiki/Self-signed_certificate) are not recommended for a production environment.**
Copy link
Contributor

Choose a reason for hiding this comment

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

Praise: It's great that you have this warning.

```{caution}
**[Self-signed certificates](https://en.wikipedia.org/wiki/Self-signed_certificate) are not recommended for a production environment.**

Check [this guide](https://charmhub.io/topics/security-with-x-509-certificates) for an overview of all the TLS certificates charms available.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Let's avoid using the word this in links if possible.

Suggested change
Check [this guide](https://charmhub.io/topics/security-with-x-509-certificates) for an overview of all the TLS certificates charms available.
Check the [Security with X.509 certificates](https://charmhub.io/topics/security-with-x-509-certificates) page for an overview of all the TLS certificates charms available.

This hands-on tutorial aims to help you learn how to deploy Charmed Valkey and
become familiar with its available operations.

## Prerequisites
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: I was expecting to see some software and/or hardware requirements here. If there are any, consider adding them here. For example: CPU, RAM, Disk, Ubuntu version, etc. Non-blocking for sure.

previously retrieved credentials:

```text
10.1.44.126:6379> AUTH charmed-operator <your-password-here>
Copy link
Contributor

Choose a reason for hiding this comment

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

Chore: Please make sure to fix code blocks (input/output combinations and cli prompts).

```

Now it is possible to perform Valkey commands on the database. To set a key `mykey`
to the value `HelloWorld`, run this command:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Wording improvement - deleting redundancy.

Suggested change
to the value `HelloWorld`, run this command:
to the value `HelloWorld`:

@@ -0,0 +1,86 @@
## Valkey operator
[![CharmHub Badge](https://charmhub.io/valkey/badge.svg)](https://charmhub.io/valkey)
[![docs badge](https://canonical-charmed-valkey.readthedocs-hosted.com/en/latest/)](https://canonical-charmed-valkey.readthedocs-hosted.com/)
Copy link
Contributor

Choose a reason for hiding this comment

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

Todo: Please fix the docs badge. You can use this as an example: https://github.com/canonical/spark-k8s-bundle/blob/main/README.md?plain=1#L5

key-value data store compatible with Redis® clients and ecosystem tooling.

The charm can be deployed on Kubernetes and VM clouds and aims to simplify Valkey
operations from Day 0 to Day 2, offering secure defaults integration interfaces,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Do you want to add a link here?

Suggested change
operations from Day 0 to Day 2, offering secure defaults integration interfaces,
operations from [Day 0 to Day 2](https://codilime.com/blog/day-0-day-1-day-2-the-software-lifecycle-in-the-cloud-age/), offering secure defaults integration interfaces,

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.

2 participants