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

[DOCS] fix a couple of typos #33356

Merged
merged 1 commit into from
Sep 4, 2018

Conversation

lonlylocly
Copy link
Contributor

Hello,

This pull request is to fix a couple of typos I spotted in the documentation.

Please tell me if there is any problem with it.

I also tried to run the docs-related checks but didn't succeed:

$ ./gradlew -p docs check

> Task :docs:integTestCluster#wait FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':docs:integTestCluster#wait'.
> Failed to start elasticsearch: timed out after 30 seconds

...

BUILD FAILED in 5m 42s
364 actionable tasks: 77 executed, 287 up-to-date

Thank you.

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

@lonlylocly Thanks for your contribution and for taking the time to help improve our documentations. These are all great catches, I will merge them soon.

Also thanks for trying to run the docs checks. I don't think they are necessary for this kind of change that doesn't change any code snippets, but I will start a full CI run anyway before merging. If you are intereted in further info about testing an building our docs please let me know, happy to help.

@cbuescher
Copy link
Member

@elasticmachine test this please

@cbuescher cbuescher merged commit d9f394b into elastic:master Sep 4, 2018
@lonlylocly
Copy link
Contributor Author

@cbuescher Thank you! Yes, I also thought that for a typo in the docs tests might not be necessary.

I will give the build another try, since I'd like to contribute to the code in the future, maybe there is a way to increase the timeout (my laptop is 3 years old and it just never happens on other developers' machines).

I suppose this is the line that times out (./docs/build.gradle:79):

  73 project.rootProject.subprojects.findAll { it.parent.path == ':plugins' }.each { subproj ->
  74   /* Skip repositories. We just aren't going to be able to test them so it
  75    * doesn't make sense to waste time installing them. */
  76   if (subproj.path.startsWith(':plugins:repository-')) {
  77     return
  78   }
  79   subproj.afterEvaluate { // need to wait until the project has been configured
  80     integTestCluster {
  81       plugin subproj.path
  82     }
  83   }

If you have any idea why this might be timing out please tell.

Thanks!

cbuescher pushed a commit that referenced this pull request Sep 4, 2018
cbuescher pushed a commit that referenced this pull request Sep 4, 2018
@cbuescher
Copy link
Member

@lonlylocly thanks, your changes should be on the master, 6.5 and 6.4 branches of the documentation soon.

maybe there is a way to increase the timeout (my laptop is 3 years old and it just never happens on other developers' machines)

Execution failed for task ':docs:integTestCluster#wait'.
> Failed to start elasticsearch: timed out after 30 seconds

This means the test framework is trying to start a small test cluster that it uses internally, and this didn't come up as expected. My first guess would be not enough resources, maybe memory or CPU is low. There are some logs written to docs/build/integTestCluster\ node[xxx]/elasticsearch-[version]/logs that might give you more info about why the cluster couldn't start, but I'm not sure its easy to debug.

I just ran ./gradlew -p docs check on my machine and it terminated at about 7 min, but this is a beefy new laptop. I must admit the tests use a lot of resources.

I will give the build another try, since I'd like to contribute to the code in the future

Sounds great, always welcome. In case you are interested in more work on the docs (which is a good start), there is https://github.com/elastic/docs that contains our tools for building the documentation or parts of it which comes with a good README. It might be slightly out of date in some places, but should be a good place to start in case you want to make bigger changes to the docs and want to test them.

jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 4, 2018
…e-default-distribution

* elastic/master: (213 commits)
  ML: Fix build after HLRC change
  Fix inner hits retrieval when stored fields are disabled (_none_) (elastic#33018)
  SQL: Show/desc commands now support table ids (elastic#33363)
  Mute testValidateFollowingIndexSettings
  HLRC: Add delete by query API (elastic#32782)
  [ML] The sort field on get records should default to the record_score (elastic#33358)
  [ML] Minor improvements to categorization Grok pattern creation (elastic#33353)
  [DOCS] fix a couple of typos (elastic#33356)
  Disable assemble task instead of removing it (elastic#33348)
  Simplify the return type of FieldMapper#parse. (elastic#32654)
  [ML] Delete forecast API (elastic#31134) (elastic#33218)
  Introduce private settings (elastic#33327)
  [Docs] Add search timeout caveats (elastic#33354)
  TESTS: Fix Race Condition in Temp Path Creation (elastic#33352)
  Fix from_range in search_after in changes snapshot (elastic#33335)
  TESTS+DISTR.: Fix testIndexCheckOnStartup Flake (elastic#33349)
  Null completion field should not throw IAE (elastic#33268)
  Adds code to help with IndicesRequestCacheIT failures (elastic#33313)
  Prevent NPE parsing the stop datafeed request. (elastic#33347)
  HLRC: Add ML get overall buckets API (elastic#33297)
  ...
@lonlylocly
Copy link
Contributor Author

lonlylocly commented Sep 5, 2018

Thanks! I checked out the logs and nothing suspicious there. Will return to this if I will happen to make another contribution.

UPDATE. Succeeded on a different, bigger machine, on the second try! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants