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.

Sign up for GitHub

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

Jump to bottom

Fix logging of wrong feature_columns in CLI and exception in auto-complete #983

Merged
merged 9 commits into from
Jun 23, 2023
Merged

Fix logging of wrong feature_columns in CLI and exception in auto-complete #983

merged 9 commits into from
Jun 23, 2023

Conversation

dagardner-nv
Copy link
Contributor

@dagardner-nv dagardner-nv commented Jun 13, 2023 •
edited
Loading

Description

The CLI command for the FIL pipeline set a default value for --columns_file flag but some FIL pipelines like ransomeware didn't use this config property, causing the default columns file to be loaded and for the values to be logged to the terminal.

  • Removed default values for --columns_file and --labels-file flags
  • Removed redundant checks for null values if they were already being enforced by required=True
  • Fix raised exception when using click auto-complete
  • Misc pylint/flake8 errors

fixes #984
fixes #819

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@dagardner-nv
Remove default value for --columns_file cli-flag, this flag isn't use…
fbdd20f 
…d by all pipelines, and causes the cli tool to mistakenly log columns not used by the pipeline
@dagardner-nv
Set --columns_file for the abp_nvsmi pipeline
bae14a8
@dagardner-nv
Remove the default value, since the flag has required=True this means…
f291a31 
… the default is never used. In addition remove checks for an unset value which will also not happen, along with the else clause which erounously set the labels
@dagardner-nv
pylint fixes
ef3812d
@dagardner-nv
flake8 fixes
c282dde
@dagardner-nv dagardner-nv requested a review from a team as a code owner June 13, 2023 19:49
@dagardner-nv dagardner-nv mentioned this pull request Jun 13, 2023
Closed
3 tasks
@dagardner-nv dagardner-nv added bug Something isn't working breaking Breaking change 3 - Ready for Review labels Jun 13, 2023
@dagardner-nv dagardner-nv requested a review from pdmack June 13, 2023 19:51
@dagardner-nv dagardner-nv changed the title Fix logging of wrong feature_columns in CLI Fix logging of wrong feature_columns in CLI and exception in auto-complete Jun 13, 2023
@mdemoret-nv
Copy link
Contributor

Waiting on review from @pdmack

pdmack
pdmack approved these changes Jun 23, 2023
View reviewed changes
Copy link
Contributor

@pdmack pdmack left a comment

Choose a reason for hiding this comment

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

LGTM

@dagardner-nv
Copy link
Contributor Author

/merge

@rapids-bot rapids-bot bot merged commit 5990f3a into nv-morpheus:branch-23.07 Jun 23, 2023
@dagardner-nv dagardner-nv deleted the david-columns-cp-819 branch June 23, 2023 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pdmack pdmack pdmack approved these changes

@mdemoret-nv mdemoret-nv mdemoret-nv approved these changes

Assignees

@dagardner-nv dagardner-nv

Labels
breaking Breaking change bug Something isn't working
Projects
Morpheus Boards
Archived in project
Milestone
No milestone
3 participants
@dagardner-nv @mdemoret-nv @pdmack