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

relax ML Info Docs expected response #38993

Merged
merged 1 commit into from
Feb 16, 2019
Merged

relax ML Info Docs expected response #38993

merged 1 commit into from
Feb 16, 2019

Conversation

talevy
Copy link
Contributor

@talevy talevy commented Feb 15, 2019

the get-ml-info API documentation tested that the
response show that ML's upgrade_mode was false.
For reasons that may be true due to other tests running in
parallel or not cleaning themselves up, this may not be
guaranteed. Since the actual value here is not of importance,
this commit relaxes the requirement that upgrade_mode be
static.

from the CI failure stacktrace

FAILURE 0.07s | DocsClientYamlTestSuiteIT.test {yaml=reference/ml/apis/get-ml-info/line_37} <<< FAILURES!
   > Throwable #1: java.lang.AssertionError: Failure at [reference/ml/apis/get-ml-info:15]: $body didn't match expected value:
   >                          $body: 
   >                         defaults: 
   >                  anomaly_detectors: 
   >        categorization_examples_limit: same [4]
   >                   model_memory_limit: same [1gb]
   >        model_snapshot_retention_days: same [1]
   >                          datafeeds: 
   >                          scroll_size: same [1000]
   >                           limits: same [empty map]
   >                     upgrade_mode: expected [false] but was [true]
   > 	at __randomizedtesting.SeedInfo.seed([3BBDE8E41A5C59B9:B3E9D73EB4A03441]:0)

there may be another doc that changes the upgrade_mode, but that should not be a concern.
The other option is to fix the leakage, but I think this change is valid on its own.

CI links to failures:

@talevy
relax ML Info Docs expected response
5cf43ea 
the get-ml-info API documentation tested that the
response show that ML's `upgrade_mode` was false.
For reasons that may be true due to other tests running in
parallel or not cleaning themselves up, this may not be
guaranteed. Since the actual value here is not of importance,
this commit relaxes the requirement that upgrade_mode be
static.
@talevy talevy added >test Issues or PRs that are addressing/adding tests >test-failure Triaged test failures from CI v7.0.0 :ml Machine learning v6.7.0 v8.0.0 v7.2.0 labels Feb 15, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

jdconrad
jdconrad approved these changes Feb 16, 2019
View reviewed changes
Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

LGTM

@talevy talevy merged commit e0e47c5 into elastic:master Feb 16, 2019
@talevy talevy deleted the fix-ml-doc-upgrademode branch February 16, 2019 00:26
talevy added a commit that referenced this pull request Feb 16, 2019
@talevy
relax ML Info Docs expected response (#38993)
cb43e1c 
the get-ml-info API documentation tested that the
response show that ML's `upgrade_mode` was false.
For reasons that may be true due to other tests running in
parallel or not cleaning themselves up, this may not be
guaranteed. Since the actual value here is not of importance,
this commit relaxes the requirement that upgrade_mode be
static.
talevy added a commit that referenced this pull request Feb 16, 2019
@talevy
relax ML Info Docs expected response (#38993)
28e80ee 
the get-ml-info API documentation tested that the
response show that ML's `upgrade_mode` was false.
For reasons that may be true due to other tests running in
parallel or not cleaning themselves up, this may not be
guaranteed. Since the actual value here is not of importance,
this commit relaxes the requirement that upgrade_mode be
static.
talevy added a commit that referenced this pull request Feb 16, 2019
@talevy
relax ML Info Docs expected response (#38993)
9275628 
the get-ml-info API documentation tested that the
response show that ML's `upgrade_mode` was false.
For reasons that may be true due to other tests running in
parallel or not cleaning themselves up, this may not be
guaranteed. Since the actual value here is not of importance,
this commit relaxes the requirement that upgrade_mode be
static.
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Feb 16, 2019
@jasontedor
Merge remote-tracking branch 'elastic/master' into retention-lease-ccr
2c77ad4 
* elastic/master:
  Ensure global test seed is used for all random testing tasks (elastic#38991)
  re-mutes SmokeTestWatcherWithSecurityIT (elastic#38995)
  Rollup jobs should be cleaned up before indices are deleted (elastic#38930)
  relax ML Info Docs expected response (elastic#38993)
  Re-enable single node tests (elastic#38852)
  ClusterClientIT refactor (elastic#38872)
  Fix typo in Index API doc
  Edits to text & formatting in Term Suggester doc (elastic#38963) (elastic#38989)
  Migrate Streamable to Writeable for WatchStatus (elastic#37390)
@lcawl lcawl mentioned this pull request Feb 19, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdconrad jdconrad jdconrad approved these changes

Assignees
No one assigned
Labels
:ml Machine learning >test Issues or PRs that are addressing/adding tests >test-failure Triaged test failures from CI v6.7.0 v7.0.0-rc2 v7.2.0 v8.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@talevy @elasticmachine @jdconrad @jakelandis