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

Added QAT configs with experimental flag. #442

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

UtkarshBhatthere
Copy link
Contributor

Description

Added cluster configs for QAT enablement

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • CleanCode (Code refactor, test updates, does not introduce functional changes)
  • Documentation update (Doc only change)

How Has This Been Tested?

Manual Testing

Contributor's Checklist

Please check that you have:

  • self-reviewed the code in this PR.
  • added code comments, particularly in hard-to-understand areas.
  • updated the user documentation with corresponding changes.
  • added tests to verify effectiveness of this change.

@UtkarshBhatthere UtkarshBhatthere force-pushed the feature/qat_enablement branch 4 times, most recently from f92aff2 to 8e60795 Compare October 15, 2024 12:53
@@ -222,6 +233,10 @@ func setConfigItem(c types.Config) error {
return err
}

if config.Permission == ClusterConfigEXPRW {
return fmt.Errorf("WARNING: set operation on experimental config (%s)", c.Key)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would advise against doing this. The pattern if err != nil is way too common in Go, and overloading errors for warnings seems like a footgun to me. Instead, I would simply log the warnings here instead of down below.

Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
Signed-off-by: Utkarsh Bhatt <utkarsh.bhatt@canonical.com>
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