Skip to content

Conversation

@lockewritesdocs
Copy link
Contributor

@lockewritesdocs lockewritesdocs commented Jul 10, 2020

@lockewritesdocs lockewritesdocs self-assigned this Jul 10, 2020
@lockewritesdocs
Copy link
Contributor Author

@jrodewig, the tests that I added are failing for both start_time_in_millis and uuid, though I can't figure out why. The build indicates that it expected one value for uuid but returned another. Shouldn't [s/s/"uuid"] substitute a value and say "don't anticipate this exact value, just match against what is in the response"?

Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

@lockewritesdocs. I left a few comments that should help fix the CI tests. Mostly minor errors that were preventing the substitutions.

@lockewritesdocs lockewritesdocs marked this pull request as ready for review July 13, 2020 19:35
@lockewritesdocs lockewritesdocs added :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >docs General docs changes Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. Team:Docs Meta label for docs team v7.8.1 v7.9.0 v8.0.0 labels Jul 13, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-docs (>docs)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Snapshot/Restore)

Copy link
Contributor

@jrodewig jrodewig left a comment

Choose a reason for hiding this comment

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

Overall LGTM.

The only blocking suggestions are that the <repository> and <snapshot> params look to be optional.

Feel free to ignore my other comments as you see fit. Thanks!

Adam Locke and others added 5 commits July 13, 2020 16:58
Copy link
Contributor

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

Thanks @lockewritesdocs looks good overall, just a few points on the details.

Also, could we add a disclaimer/warning to this API that makes it clear that this might be very expensive to execute if you run it over a large number of snapshots or over snapshots that contain a large number of shards? People should really not be using this API as part of e.g. orchestration logic unless absolutely necessary IMO.

Copy link
Contributor

@original-brownbear original-brownbear left a comment

Choose a reason for hiding this comment

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

LGTM thanks for the extra iterations @lockewritesdocs !

@lockewritesdocs lockewritesdocs merged commit 556f19d into elastic:master Jul 15, 2020
@lockewritesdocs lockewritesdocs deleted the docs__get-snapshot-status-api-docs branch July 15, 2020 20:28
lockewritesdocs pushed a commit to lockewritesdocs/elasticsearch that referenced this pull request Jul 15, 2020
* Adding get snapshot status API docs.

* Adding more fields and a link to the new page.

* Adding missing spaces in TESTRESPONSES

* Adding more parameters and making some edits.

* Marking snapshot as optional

* Marking repository as optional

* Add data type for stats

* Add data type for shard_stats

* Incorporating review feedback.

* Lots of review feedback incorporated.

* Fixing tests to unbreak CI builds.

* Changing indices to index.
lockewritesdocs pushed a commit that referenced this pull request Jul 16, 2020
* Adding get snapshot status API docs.

* Adding more fields and a link to the new page.

* Adding missing spaces in TESTRESPONSES

* Adding more parameters and making some edits.

* Marking snapshot as optional

* Marking repository as optional

* Add data type for stats

* Add data type for shard_stats

* Incorporating review feedback.

* Lots of review feedback incorporated.

* Fixing tests to unbreak CI builds.

* Changing indices to index.
lockewritesdocs pushed a commit to lockewritesdocs/elasticsearch that referenced this pull request Jul 16, 2020
* Adding get snapshot status API docs.

* Adding more fields and a link to the new page.

* Adding missing spaces in TESTRESPONSES

* Adding more parameters and making some edits.

* Marking snapshot as optional

* Marking repository as optional

* Add data type for stats

* Add data type for shard_stats

* Incorporating review feedback.

* Lots of review feedback incorporated.

* Fixing tests to unbreak CI builds.

* Changing indices to index.
lockewritesdocs pushed a commit to lockewritesdocs/elasticsearch that referenced this pull request Jul 16, 2020
* Adding get snapshot status API docs.

* Adding more fields and a link to the new page.

* Adding missing spaces in TESTRESPONSES

* Adding more parameters and making some edits.

* Marking snapshot as optional

* Marking repository as optional

* Add data type for stats

* Add data type for shard_stats

* Incorporating review feedback.

* Lots of review feedback incorporated.

* Fixing tests to unbreak CI builds.

* Changing indices to index.
lockewritesdocs pushed a commit that referenced this pull request Jul 16, 2020
* Adding get snapshot status API docs.

* Adding more fields and a link to the new page.

* Adding missing spaces in TESTRESPONSES

* Adding more parameters and making some edits.

* Marking snapshot as optional

* Marking repository as optional

* Add data type for stats

* Add data type for shard_stats

* Incorporating review feedback.

* Lots of review feedback incorporated.

* Fixing tests to unbreak CI builds.

* Changing indices to index.
lockewritesdocs pushed a commit that referenced this pull request Jul 16, 2020
* Adding get snapshot status API docs.

* Adding more fields and a link to the new page.

* Adding missing spaces in TESTRESPONSES

* Adding more parameters and making some edits.

* Marking snapshot as optional

* Marking repository as optional

* Add data type for stats

* Add data type for shard_stats

* Incorporating review feedback.

* Lots of review feedback incorporated.

* Fixing tests to unbreak CI builds.

* Changing indices to index.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >docs General docs changes Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. Team:Docs Meta label for docs team v7.8.1 v7.9.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants