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

Documentation: Add ssl_context option to RedisCluster documentation #300

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

boesing
Copy link
Member

@boesing boesing commented Apr 16, 2024

Q A
Documentation yes

Description

This adds the documentation of the new created ssl_options option flag for RedisCluster options.
Closes #299
Is a new patch version feasible, this component does actually not change at all.

@boesing boesing added this to the 3.12.2 milestone Apr 16, 2024
@boesing boesing requested a review from froschdesign April 16, 2024 09:32
@boesing boesing linked an issue Apr 16, 2024 that may be closed by this pull request
@boesing boesing force-pushed the docs/redis-cluster-ssl-context branch from b9924f9 to cb716eb Compare April 16, 2024 09:34
| Name | Data Type | Default Value | Description |
|-----------------------|---------------------------|---------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `lib_options` | `array` | `[]` | Associative array of Redis options where the array key is the options constant value (see `RedisCluster::OPT_*` [constants](https://github.com/JetBrains/phpstorm-stubs/blob/master/redis/RedisCluster.php) for details). |
| `namespace_separator` | `string` | ":" | A separator for the namespace and prefix. |
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
| `namespace_separator` | `string` | ":" | A separator for the namespace and prefix. |
| `namespace_separator` | `string` | `:` | A separator for the namespace and prefix. |

|-----------------------|---------------------------|---------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `lib_options` | `array` | `[]` | Associative array of Redis options where the array key is the options constant value (see `RedisCluster::OPT_*` [constants](https://github.com/JetBrains/phpstorm-stubs/blob/master/redis/RedisCluster.php) for details). |
| `namespace_separator` | `string` | ":" | A separator for the namespace and prefix. |
| `password` | `string` | "" | Password to authenticate with Redis server |
Copy link
Member

Choose a reason for hiding this comment

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

I suggest a comment like "(empty)" as the default value.

| `lib_options` | `array` | `[]` | Associative array of Redis options where the array key is the options constant value (see `RedisCluster::OPT_*` [constants](https://github.com/JetBrains/phpstorm-stubs/blob/master/redis/RedisCluster.php) for details). |
| `namespace_separator` | `string` | ":" | A separator for the namespace and prefix. |
| `password` | `string` | "" | Password to authenticate with Redis server |
| `name` | `string` | "" | Name to determine configuration from [php.ini](https://github.com/phpredis/phpredis/blob/develop/cluster.markdown#loading-a-cluster-configuration-by-name) (**MUST NOT** be combined with `seeds`) |
Copy link
Member

Choose a reason for hiding this comment

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

The same suggestion here.

| `timeout` | `float` | `1.0` | Timeout for commands, see [PhpRedis](https://github.com/phpredis/phpredis/blob/develop/cluster.markdown#timeouts) timeouts documentation for more background. |
| `read_timeout` | `float` | `2.0` | Read timeout for commands, see [PhpRedis](https://github.com/phpredis/phpredis/blob/develop/cluster.markdown#timeouts) timeouts documentation for more background. |
| `persistent` | `bool` | `false` | Flag to specify whether to create a persistent connection or not |
| `version` | `string` | "" | The Redis server version. **MUST** be specified in a [Semantic Versioning 2.0.0](https://semver.org/#semantic-versioning-200) format. This information is used to determine some features/capabilities without opening a connection to the server. |
Copy link
Member

Choose a reason for hiding this comment

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

And here.

@boesing
Copy link
Member Author

boesing commented Jun 14, 2024

@froschdesign I keep the suggestions here open as they should be done indeed but actually only appeared due to the fact that I reformatted all the tables to match markdown codingstyle. Contents where not changed by me and thus lets keep this focused on adding ssl_context.

I am still unhappy with the way how adapter configuration has to be maintained.
I raised this concern a lot yet I am unable to provide useful input.
The main pain points for me are:

  • cross repository changes
  • changes which do not reflect adapter versioning (there are multiple majors in several adapters)

I'd like to still see all the docs regarding an adapter listed in the appropriate satellites. So we can provide dedicated cache page the same way we do for https://docs.laminas.dev/mvc/
This would allow us to have every (supported) adapter listed on that page individually in dedicated versionings.
No idea how we did that for MVC but IMHO it should look like that so that we finally can provide feature documentation from within the adapter repository and do not have cross repository PRs such as these.

This said, I will merge this for now so that we have the SSL context properly documented.

@boesing boesing merged commit f99d10d into laminas:3.12.x Jun 14, 2024
2 of 5 checks passed
@boesing boesing deleted the docs/redis-cluster-ssl-context branch June 14, 2024 13:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add documentation to RedisCluster that we allow ssl_context option
2 participants