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

[ML] Fix spaces job ID check #84404

Merged
merged 2 commits into from
Nov 30, 2020

Conversation

jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Nov 26, 2020

Fixes bug where anomaly detection endpoints would not return a 404 when a job exists in a different space but not the current space.

GET /api/ml/anomaly_detectors/foo should 404 if foo exists but is in a different space.
GET /api/ml/anomaly_detectors/foo* should should return an empty list of jobs.

cc @pheyos

Checklist

@jgowdyelastic jgowdyelastic added bug Fixes for quality problems that affect the customer experience non-issue Indicates to automation that a pull request should not appear in the release notes :ml Feature:Anomaly Detection ML anomaly detection release_note:skip Skip the PR/issue when compiling release notes labels Nov 26, 2020
@jgowdyelastic jgowdyelastic self-assigned this Nov 26, 2020
@jgowdyelastic jgowdyelastic requested a review from a team as a code owner November 26, 2020 11:45
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

Copy link
Member

@pheyos pheyos left a comment

Choose a reason for hiding this comment

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

LGTM
Validated that the following is working with this PR:
GET /api/ml/anomaly_detectors/foo should 404 if foo exists but is in a different space.
GET /api/ml/anomaly_detectors/foo* should should return an empty list of jobs.
The other tests that I have in my development branch so far also passed with this PR.

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

LGTM

@pheyos
Copy link
Member

pheyos commented Nov 30, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@peteharverson peteharverson merged commit 767286e into elastic:master Nov 30, 2020
peteharverson pushed a commit to peteharverson/kibana that referenced this pull request Nov 30, 2020
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 30, 2020
* master:
  [Security Solution] Exceptions Cypress tests (elastic#81759)
  [ML] Fix spaces job ID check (elastic#84404)
  [Security Solution][Detections] Handle dupes when processing threshold rules (elastic#83062)
  skip flaky suite (elastic#84440)
  skip flaky suite (elastic#84445)
  [APM] Fix missing `service.node.name` (elastic#84269)
  Upgrade fp-ts to 2.8.6 (elastic#83866)
  Added data streams privileges to better control delete actions in UI (elastic#83573)
  Improve short-url redirect validation (elastic#84366)
  TSVB offsets (elastic#83051)
  [Discover] Fix navigating back when changing index pattern (elastic#84061)
  [Logs UI] Polish the UI for the log entry examples in the anomaly table (elastic#82139)
  [Logs UI] Limit the height of the "view in context" container (elastic#83178)
  [Application Usage] Update `schema` with new `fleet` rename (elastic#84327)
  fix identation in list (elastic#84301)
peteharverson added a commit that referenced this pull request Nov 30, 2020
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: James Gowdy <jgowdy@elastic.co>
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
phillipb added a commit to phillipb/kibana that referenced this pull request Nov 30, 2020
…bana into add-metadata-to-node-details

* 'add-metadata-to-node-details' of github.com:phillipb/kibana:
  [APM] ML anomaly detection integration: Displaying anomaly job results in the Transaction duration chart is not as intended  (elastic#84415)
  Support for painless language autocomplete within monaco (elastic#80577)
  [Lens] Time scale ui (elastic#83904)
  removing beta callouts (elastic#84510)
  [Lens] (Accessibility) add aria-label to chart type icon (elastic#84493)
  Trusted Apps signer API. (elastic#83661)
  increase stdout max listeners for legacy logging (elastic#84497)
  [APM] Service overview: Add throughput chart (elastic#84439)
  [Discover] Unskip main functional tests (elastic#84300)
  Uptime overview overhaul (elastic#83406)
  [APM] Adjust time formats based on the difference between start and end (elastic#84470)
  [ML] Renaming saved object repair to sync (elastic#84311)
  [UsageCollection] Remove `formatBulkUpload` and other unused APIs (elastic#84313)
  [Visualizations] Adds visConfig.title and uiState to build pipeline function (elastic#84456)
  [Elasticsearch Migration] Update docs re UsageCollection (elastic#84322)
  TSVB field list performance issue on using annotations (elastic#84407)
  [Security Solution] Exceptions Cypress tests (elastic#81759)
  [ML] Fix spaces job ID check (elastic#84404)
  [Security Solution][Detections] Handle dupes when processing threshold rules (elastic#83062)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:Anomaly Detection ML anomaly detection :ml non-issue Indicates to automation that a pull request should not appear in the release notes release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants