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

Breaking up envvars and config settings into separate settings pages #1028

Merged
merged 14 commits into from
Nov 3, 2023

Conversation

djwfyi
Copy link
Collaborator

@djwfyi djwfyi commented Oct 6, 2023

Creates a slew of folders and pages nested under the mc-server page to store settings.
Breaks up all of the environment variable options formerly in the mc-server page into these separate pages.
Moves the config settings formerly in the mc admin config page to the appropriate new settings pages.

Closes #1017

Staged:
http://192.241.195.202:9000/staging/server-breakup/linux/reference/minio-server/settings.html

@djwfyi djwfyi requested review from feorlen and ravindk89 October 6, 2023 20:58
@djwfyi djwfyi self-assigned this Oct 6, 2023
@feorlen
Copy link
Collaborator

feorlen commented Oct 6, 2023

The TOC order feels a bit arbitrary? As in, I would expect things like core and root near the top and additional at the end. (The ordering in the original page wasn't wonderful, to be sure.)

Maybe something like (or thereabouts)

core
root
console
storage class
iam
metrics/logging
kes
notifications
additional


Configuration Settings
----------------------

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe something here that links to mc admin config? If you randomly showed up on this page it's a pointer to how they are used.

@ravindk89
Copy link
Collaborator

Hm. So Open Question ™️ on the structure.

Right now it's:

# Environment Variables

## Thing A

# Configuration Settings

## Thing A

I would actually suggestoin


# Thing A Settings

## Environment Variables

## Configuration Settings

That way all of the possible settings for a given Thing are in one scrollable H1. Thoughts?

Copy link
Collaborator

@ravindk89 ravindk89 left a comment

Choose a reason for hiding this comment

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

Most of my thoughts are high level org/toc related, and only a few word-level changes (for now).

source/reference/minio-server/settings/console.rst Outdated Show resolved Hide resolved
source/reference/minio-server/settings/console.rst Outdated Show resolved Hide resolved
source/reference/minio-server/settings/core.rst Outdated Show resolved Hide resolved
source/reference/minio-server/settings/notifications.rst Outdated Show resolved Hide resolved
@djwfyi djwfyi requested review from feorlen and ravindk89 November 1, 2023 15:11
Copy link
Collaborator

@ravindk89 ravindk89 left a comment

Choose a reason for hiding this comment

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

This is like 90% solid work Daryl. Very impressive.

source/reference/minio-server/settings/core.rst Outdated Show resolved Hide resolved

*Optional*

Enable parallel workers by specifying the maximum number of processes to use when performing the batch application job.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe the workers are always parallel. This just sets the number of workers.

@poornas do we have guidance here on how to set it, or what the defaults are?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated the description, but can add defaults or other recommendations if we have any.

Copy link
Contributor

Choose a reason for hiding this comment

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

batch workers can be set with _MINIO_BATCH_REPLICATION_WORKERS - it defaults to max cpus/2

source/reference/minio-server/settings/storage-class.rst Outdated Show resolved Hide resolved
Settings
--------

Enable
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm. I'm guessing we have to do it this way to avoid the duplication of text the entire way down for each tab item?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is having an H2 for every single setting

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also provides nice in-page navigation (on the right toc) to jump to a specific setting.

source/reference/minio-server/settings/kes.rst Outdated Show resolved Hide resolved
source/reference/minio-server/settings/deprecated.rst Outdated Show resolved Hide resolved

``access``
For each bucket event, MinIO creates a JSON document with the event details and appends it to the key with a Redis-generated random ID.
Additional updates to an object result in new index entries, and existing entries remain unmodified.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Additional updates to an object result in new index entries, and existing entries remain unmodified.
Additional updates to an object result in new index entries, and existing entries remain unmodified.

Copy link
Collaborator

@feorlen feorlen left a comment

Choose a reason for hiding this comment

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

Noted a few items to look at, along with a bunch of opportunistic fixes to the original text. This is a huge undertaking and already looks so much better.

I think your idea to use an include for the nothing to see here tab text would make future-writer very happy. Yay.

Copy link
Collaborator

@feorlen feorlen left a comment

Choose a reason for hiding this comment

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

👏🏻 🎊

Copy link
Collaborator

@ravindk89 ravindk89 left a comment

Choose a reason for hiding this comment

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

very minor nits. Done!

source/reference/minio-server/settings/core.rst Outdated Show resolved Hide resolved
source/reference/minio-mc/minio-client-settings.rst Outdated Show resolved Hide resolved
@djwfyi djwfyi merged commit eb94513 into main Nov 3, 2023
@djwfyi djwfyi deleted the server-breakup branch November 3, 2023 11:55
@djwfyi djwfyi mentioned this pull request Nov 3, 2023
djwfyi added a commit that referenced this pull request Nov 3, 2023
Corrects errors from `mc-conf` references not noticed until after
merging #1028 .

No issue to track it.
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.

Break out MinIO Server reference page to have child pages for envvars + configs
4 participants