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

fix: Compress applies to all storage modes, SyncInterval only to persistent #58

Merged
merged 4 commits into from
Jan 16, 2024

Conversation

richm
Copy link
Contributor

@richm richm commented Jan 13, 2024

Compress should apply to all storage modes

SyncInterval should apply only to persistent mode, and warn otherwise

Reformat the journald.conf template to make it easier to see which
settings apply to which modes.

Add test for ForwardToSyslog

Ensure the test cleans up properly

Signed-off-by: Rich Megginson [email protected]

@richm richm requested a review from spetrosi as a code owner January 13, 2024 17:30
@richm
Copy link
Contributor Author

richm commented Jan 13, 2024

[citest]

@richm richm requested a review from msekletar January 13, 2024 17:33
Copy link
Collaborator

@msekletar msekletar left a comment

Choose a reason for hiding this comment

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

Nitpick, there is a bunch unrelated changes in a single commit. Maybe it would be better to split it.

Regarding, SyncInterval=. What is the point of calling sync() at all? For the most part, It is to ensure data persistence but there no persistence in volatile mode. So we should probably warn if SyncInterval is set in volatile mode because it is probably an error.

@richm
Copy link
Contributor Author

richm commented Jan 15, 2024

Nitpick, there is a bunch unrelated changes in a single commit. Maybe it would be better to split it.

Regarding, SyncInterval=. What is the point of calling sync() at all? For the most part, It is to ensure data persistence but there no persistence in volatile mode. So we should probably warn if SyncInterval is set in volatile mode because it is probably an error.

ok - check out the latest commits

@richm richm changed the title fix: Compress and SyncInterval should apply to all storage modes fix: Compress applies to all storage modes, SyncInterval only to persistent Jan 15, 2024
@richm
Copy link
Contributor Author

richm commented Jan 15, 2024

[citest]

Copy link
Collaborator

@msekletar msekletar left a comment

Choose a reason for hiding this comment

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

LGTM

@richm richm merged commit a4a8ee7 into linux-system-roles:main Jan 16, 2024
22 checks passed
@richm richm deleted the fixes branch January 16, 2024 14:36
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