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

Provide Deprecate Warnings for Experimental ML commands #4365

Merged
merged 14 commits into from
Jan 15, 2025

Conversation

shashank-elastic
Copy link
Contributor

@shashank-elastic shashank-elastic commented Jan 10, 2025

Pull Request

Issue link(s): #4023

Summary - What I changed

  • Deprecate the ML command and finally remove them was the task.
  • Initial thoughts was to use warnings as a structure, when code was explored experimental group commands had a click.secho warning structure (refer)
  • The same is used to annouce ml group commands deprecation.
  • We can release this as a minor change and then once removed we can push a major ( else we can use a patch / minor )
  • We can also give a window of 1 week say max 2 weeks to remove the command since its not actively used!

How To Test

Local Testing

python -m detection_rules es  experimental ml check-files
Loaded config file: /Users/shashankks/elastic_workspace/detection-rules/.detection-rules-cfg.json

█▀▀▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄   ▄      █▀▀▄ ▄  ▄ ▄   ▄▄▄ ▄▄▄
█  █ █▄▄  █  █▄▄ █    █   █  █ █ █▀▄ █      █▄▄▀ █  █ █   █▄▄ █▄▄
█▄▄▀ █▄▄  █  █▄▄ █▄▄  █  ▄█▄ █▄█ █ ▀▄█      █ ▀▄ █▄▄█ █▄▄ █▄▄ ▄▄█


* experimental commands are use at your own risk and may change without warning *


***** Deprecation Warning *****


* The experiment "ml" command(s) are deprecated and will be removed in a future release. *

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/Users/shashankks/elastic_workspace/detection-rules/detection_rules/__main__.py", line 35, in <module>
    main()
  File "/Users/shashankks/elastic_workspace/detection-rules/detection_rules/__main__.py", line 32, in main
    root(prog_name="detection_rules")
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  [Previous line repeated 1 more time]
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/decorators.py", line 33, in new_func
    return f(get_current_context(), *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/detection_rules/ml.py", line 286, in check_files
    files = MachineLearningClient.get_all_ml_files(ctx.obj['es'])
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/detection_rules/ml.py", line 203, in get_all_ml_files
    scripts = es_client.cluster.state()['metadata']['stored_scripts']
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
KeyError: 'stored_scripts'
(.venv) 
detection-rules on  issue-4023 [$?] is 📦 v0.3.9 via 🐍 v3.12.5 (.venv) on ☁️  shashank.suryanarayana@elastic.co took 3s 

The same is used in all subcommands as its mentioned in group command, trying another subcommand

python -m detection_rules es  experimental ml upload-job
Loaded config file: /Users/shashankks/elastic_workspace/detection-rules/.detection-rules-cfg.json

█▀▀▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄   ▄      █▀▀▄ ▄  ▄ ▄   ▄▄▄ ▄▄▄
█  █ █▄▄  █  █▄▄ █    █   █  █ █ █▀▄ █      █▄▄▀ █  █ █   █▄▄ █▄▄
█▄▄▀ █▄▄  █  █▄▄ █▄▄  █  ▄█▄ █▄█ █ ▀▄█      █ ▀▄ █▄▄█ █▄▄ █▄▄ ▄▄█


* experimental commands are use at your own risk and may change without warning *


***** Deprecation Warning *****


* The experiment "ml" command(s) are deprecated and will be removed in a future release. *

Usage: detection_rules es experimental ml upload-job [OPTIONS] JOB_FILE
Try 'detection_rules es experimental ml upload-job -h' for help.

Error: Missing argument 'JOB_FILE'.
(.venv) 
detection-rules on  issue-4023 [$?] is 📦 v0.3.9 via 🐍 v3.12.5 (.venv) on ☁️  shashank.suryanarayana@elastic.co 

Local Testing After review comments to add date and time

python -m detection_rules es  experimental ml check-files
Loaded config file: /Users/shashankks/elastic_workspace/detection-rules/.detection-rules-cfg.json

█▀▀▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄▄▄ ▄   ▄      █▀▀▄ ▄  ▄ ▄   ▄▄▄ ▄▄▄
█  █ █▄▄  █  █▄▄ █    █   █  █ █ █▀▄ █      █▄▄▀ █  █ █   █▄▄ █▄▄
█▄▄▀ █▄▄  █  █▄▄ █▄▄  █  ▄█▄ █▄█ █ ▀▄█      █ ▀▄ █▄▄█ █▄▄ █▄▄ ▄▄█


* experimental commands are use at your own risk and may change without warning *


***** Deprecation Warning *****


* The experiment "ml" command(s) are deprecated and will be removed in a future release. *


* Command Removal Timeframe May 1 2025 *

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/Users/shashankks/elastic_workspace/detection-rules/detection_rules/__main__.py", line 35, in <module>
    main()
  File "/Users/shashankks/elastic_workspace/detection-rules/detection_rules/__main__.py", line 32, in main
    root(prog_name="detection_rules")
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  [Previous line repeated 1 more time]
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/.venv/lib/python3.12/site-packages/click/decorators.py", line 33, in new_func
    return f(get_current_context(), *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/detection_rules/ml.py", line 288, in check_files
    files = MachineLearningClient.get_all_ml_files(ctx.obj['es'])
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/shashankks/elastic_workspace/detection-rules/detection_rules/ml.py", line 203, in get_all_ml_files
    scripts = es_client.cluster.state()['metadata']['stored_scripts']
              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
KeyError: 'stored_scripts'
(.venv) 
detection-rules on  issue-4023 [$!?] is 📦 v0.3.15 via 🐍 v3.12.5 (.venv) on ☁️  shashank.suryanarayana@elastic.co took 2s 

Checklist

  • Added a label for the type of pr: bug, enhancement, schema, maintenance, Rule: New, Rule: Deprecation, Rule: Tuning, Hunt: New, or Hunt: Tuning so guidelines can be generated
  • Added the meta:rapid-merge label if planning to merge within 24 hours
  • Secret and sensitive material has been managed correctly
  • Automated testing was updated or added to match the most common scenarios
  • Documentation and comments were added for features that require explanation

Contributor checklist

@shashank-elastic shashank-elastic self-assigned this Jan 10, 2025
@botelastic botelastic bot added the python Internal python for the repository label Jan 10, 2025
@shashank-elastic shashank-elastic linked an issue Jan 10, 2025 that may be closed by this pull request
@shashank-elastic shashank-elastic added the enhancement New feature or request label Jan 10, 2025
Copy link
Contributor

Enhancement - Guidelines

These guidelines serve as a reminder set of considerations when addressing adding a feature to the code.

Documentation and Context

  • Describe the feature enhancement in detail (alternative solutions, description of the solution, etc.) if not already documented in an issue.
  • Include additional context or screenshots.
  • Ensure the enhancement includes necessary updates to the documentation and versioning.

Code Standards and Practices

  • Code follows established design patterns within the repo and avoids duplication.
  • Code changes do not introduce new warnings or errors.
  • Variables and functions are well-named and descriptive.
  • Any unnecessary / commented-out code is removed.
  • Ensure that the code is modular and reusable where applicable.
  • Check for proper exception handling and messaging.

Testing

  • New unit tests have been added to cover the enhancement.
  • Existing unit tests have been updated to reflect the changes.
  • Provide evidence of testing and validating the enhancement (e.g., test logs, screenshots).
  • Validate that any rules affected by the enhancement are correctly updated.
  • Ensure that performance is not negatively impacted by the changes.
  • Verify that any release artifacts are properly generated and tested.

Additional Checks

  • Ensure that the enhancement does not break existing functionality.
  • Review the enhancement with a peer or team member for additional insights.
  • Verify that the enhancement works across all relevant environments (e.g., different OS versions).
  • Confirm that all dependencies are up-to-date and compatible with the changes.
  • Confirm that the proper version label is applied to the PR patch, minor, major.

@shashank-elastic shashank-elastic changed the title Deprecate Experimental ML commands Provide Deprecate Warnings for Experimental ML commands Jan 13, 2025
Co-authored-by: Eric Forte <119343520+eric-forte-elastic@users.noreply.github.com>
@shashank-elastic shashank-elastic merged commit 32f5966 into main Jan 15, 2025
12 checks passed
@shashank-elastic shashank-elastic deleted the issue-4023 branch January 15, 2025 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport: auto community enhancement New feature or request patch python Internal python for the repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FR] Deprecate Experimental ML Logic
4 participants