Skip to content

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