Skip to content

Add Verifier developer guide to presto doc #12686

Closed
caithagoras wants to merge 1 commit intoprestodb:masterfrom
caithagoras:s2
Closed

Add Verifier developer guide to presto doc #12686
caithagoras wants to merge 1 commit intoprestodb:masterfrom
caithagoras:s2

Conversation

@caithagoras
Copy link
Contributor

No description provided.

Copy link

@mayankgarg1990 mayankgarg1990 left a comment

Choose a reason for hiding this comment

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

It will be great if @rschlussel can also take a look since her English grammar knowledge is much better than mine :)

@caithagoras caithagoras force-pushed the s2 branch 2 times, most recently from b4d370e to bccd7cc Compare April 17, 2019 21:33
@caithagoras caithagoras changed the title Add Verifier Mechanics to presto doc Add Verifier developer guide to presto doc Apr 17, 2019
Copy link
Contributor

@rschlussel rschlussel left a comment

Choose a reason for hiding this comment

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

This documentation lists all kinds of configurations that can be set and plugins that can be added but the user documentation doesn't have explanations for how to do most of these things. I think you should expand the user documentation to include descriptions of how to do all the things described here and also to include more fleshed out overview of what the verifier does and how it works so people can understand what they are configuring.

I don't know what our guidelines are for user vs. developer documentation, but nearly everything in this PR seems suitable for user documentation. I'm not sure we'll need developer documentation if the user documentation is more thorough.

Copy link
Contributor

Choose a reason for hiding this comment

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

e.g. not i.e.. But I would elaborate that by default Global and time limit exceeded are the only failures that are automatically resolved.
Also, I'm curious whether you can turn off automatic resolution of those failures (e.g. if you have the same cluster set up for both, I imagine people may not want those automatically resolved).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't have the switch yet but it make sense to me to add one.

@caithagoras
Copy link
Contributor Author

caithagoras commented Apr 29, 2019

@rschlussel I wonder what you are referring to when use said "user documentation"? I though the developer guide is part of the user documentation but is more towards power users.

Currently we have a section about verifier in the installation section (http://prestodb.github.io/docs/current/installation/verifier.html), which contains a list of commonly-used configuration properties. How would you recommend we organize those information.

@rschlussel
Copy link
Contributor

@caithagoras I didn't realize at first that it was in a section of the user documentation. But either way my point is that all the documentation should be together in one place. Right now there's "beginner" information in one place that isn't quite enough to understand what the verifier is or how to use it. and then there's "advanced" information in another place that explains more background about how it works and things you can do, but doesn't tell you how to do them or where you could learn more. I think it would be much clearer for both kinds of users to have all the information in one place. I don't think it matters whether it goes in the installation section or the developer section.

@mbasmanova
Copy link
Contributor

@caithagoras This documentation would be very useful for folks who are running the verifiers and investigating the results. It is helpful to understand what kind of things are resolved automatically and what are not. Let's address the comments and land this doc.

CC: @nezihyigitbasi

@caithagoras caithagoras deleted the s2 branch January 23, 2020 00:55
@caithagoras caithagoras restored the s2 branch January 23, 2020 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants