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

docs: Improve archive log docs as to why it's not recommended #12597

Merged
merged 7 commits into from
Feb 21, 2024

Conversation

Joibel
Copy link
Member

@Joibel Joibel commented Jan 31, 2024

And document how to make life easier with links.

Switch to an admonition as per #12561.

Header:
image

And then new documentation

Screenshot 2024-02-25 at 10 15 20 AM

Verification

mkdocs build

See screenshots above.

@Joibel Joibel added the area/docs Incorrect, missing, or mistakes in docs label Jan 31, 2024
And document how to make life easier with links.

Signed-off-by: Alan Clucas <alan@clucas.org>
@agilgur5 agilgur5 self-assigned this Jan 31, 2024
@agilgur5 agilgur5 added the area/artifacts S3/GCP/OSS/Git/HDFS etc label Jan 31, 2024
@agilgur5
Copy link
Member

I'm guessing this is a follow-up to the question and my answer in #12593?

I haven't gotten that question too often, but good to have a longer note on it. In particular Argo seems to get a good bit of users who aren't necessarily operators/k8s cluster-admins and so may not know about fluentd, Splunk, ELK etc.

Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

Recommending some structural changes for more directness and changing the verbal descriptions of links to example YAML. Also some changes per the style guide

docs/configure-archive-logs.md Outdated Show resolved Hide resolved
docs/configure-archive-logs.md Outdated Show resolved Hide resolved
docs/configure-archive-logs.md Outdated Show resolved Hide resolved
docs/configure-archive-logs.md Outdated Show resolved Hide resolved
docs/configure-archive-logs.md Outdated Show resolved Hide resolved
@Joibel
Copy link
Member Author

Joibel commented Feb 1, 2024

I'm guessing this is a follow-up to the question and my answer in #12593?

I haven't gotten that question too often, but good to have a longer note on it. In particular Argo seems to get a good bit of users who aren't necessarily operators/k8s cluster-admins and so may not know about fluentd, Splunk, ELK etc.

Yes, directly inspired by that. It's a common question in face-to-face chats with argo users, so I thought worth addressing with a long term fix.

Signed-off-by: Alan Clucas <alan@clucas.org>
@Joibel
Copy link
Member Author

Joibel commented Feb 1, 2024

Screenshots at the top of the issue are updated with the latest changes.

@Joibel
Copy link
Member Author

Joibel commented Feb 8, 2024

@agilgur5 could you check you're happy with this?

@agilgur5 agilgur5 added area/archive-logs Archive Logs feature and removed area/artifacts S3/GCP/OSS/Git/HDFS etc labels Feb 8, 2024
Copy link
Member

@agilgur5 agilgur5 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 better, but there are still some pieces from the old version remaining that are now quite repetitive (and themselves still have style issues). Re-ordering it as suggested below will consolidate that significantly

docs/configure-archive-logs.md Outdated Show resolved Hide resolved
docs/configure-archive-logs.md Outdated Show resolved Hide resolved
docs/configure-archive-logs.md Outdated Show resolved Hide resolved
docs/configure-archive-logs.md Outdated Show resolved Hide resolved
@ljyanesm
Copy link

ljyanesm commented Feb 9, 2024

Is there a way to reconfigure the UI after having followed these new recommendations to disable the default logs button and possibly the other warnings and references to logs?

Seems a bit awkward to potentially have multiple buttons for logs where some of them won't resolve but would still be enabled.

Signed-off-by: Alan Clucas <alan@clucas.org>
@Joibel
Copy link
Member Author

Joibel commented Feb 12, 2024

Screenshot updated at the top.

Joibel and others added 2 commits February 20, 2024 12:28
Co-authored-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
Signed-off-by: Alan Clucas <alan@clucas.org>
@agilgur5
Copy link
Member

agilgur5 commented Feb 21, 2024

Screenshot updated at the top.

This needs updating once more, but otherwise LGTM. EDIT: I updated it myself now that I have that permission

Copy link
Member

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

LGTM; addresses a common question

@agilgur5 agilgur5 merged commit 11b701a into argoproj:main Feb 21, 2024
15 checks passed
agilgur5 pushed a commit that referenced this pull request May 4, 2024
Signed-off-by: Alan Clucas <alan@clucas.org>
Co-authored-by: Anton Gilgur <4970083+agilgur5@users.noreply.github.com>
(cherry picked from commit 11b701a)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/archive-logs Archive Logs feature area/docs Incorrect, missing, or mistakes in docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants