-
-
Notifications
You must be signed in to change notification settings - Fork 0
Fix technical debts #20
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
base: main
Are you sure you want to change the base?
Conversation
@@ -185,7 +185,6 @@ impl IsLabelValue for ValidatedCluster { | |||
} | |||
} | |||
|
|||
// TODO Remove boilerplate (like derive_more) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to leave it as is instead of adding another crate just for this one case.
}; | ||
|
||
pub const CONFIGURATION_FILE_OPENSEARCH_YML: &str = "opensearch.yml"; | ||
|
||
// TODO Document how to enter config_overrides of various types, e.g. string, list, boolean, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done in #14
@@ -49,8 +49,6 @@ impl<'a> RoleBuilder<'a> { | |||
} | |||
} | |||
|
|||
// TODO Only one builder function which calls the other ones? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is okay as it is.
DISABLE_INSTALL_DEMO_CONFIG: "true" | ||
OPENSEARCH_HOME: {{ test_scenario['values']['opensearch_home'] }} | ||
configOverrides: | ||
# TODO Add the required options to the operator |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are follow-up tickets. No need for these TODOs in the code.
opensearch.yml: | ||
# Disable memory mapping in this test; If memory mapping were activated, the kernel setting | ||
# vm.max_map_count would have to be increased to 262144 on the node. | ||
node.store.allow_mmap: "false" | ||
# TODO Check that this is safe despite the warning in the documentation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is safe if the demo configuration is replaced with podOverrides
. To be on the safe side, I removed the demo configuration in stackabletech/docker-images#1228.
@@ -1,7 +1,6 @@ | |||
# All fields are checked that are set by the operator. | |||
# This helps to detect unintentional changes. | |||
# The maintenance effort should be okay as long as it is only done in the smoke test. | |||
# TODO Check individual field in unit tests |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a follow-up task in #2.
Description
The remaining TODOs are fixed or removed if not necessary anymore.
Definition of Done Checklist
Author
Reviewer
Acceptance
type/deprecation
label & add to the deprecation scheduletype/experimental
label & add to the experimental features tracker