-
Notifications
You must be signed in to change notification settings - Fork 783
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
Documentation Revamp #496
Documentation Revamp #496
Conversation
Thanks @vishal-biyani. Would you mind putting the structure in the PR description too? @life1347 @timirahj please take a look and comment if you have any ideas/suggestions around the doc structure. |
Below structure and basic content are targeted as part of this PR.
|
@@ -0,0 +1,6 @@ | |||
--- | |||
title: "Executor" | |||
date: 2017-09-07T20:10:05-07:00 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Date seems to be off in many of the documents, I would propose to just set it to today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I am not sure if Hugo has a better way, adding TS manually seems odd to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay, I was not aware this was generated by Hugo. It seems to be just a ISO 8601 datetime, so probably your editor has an option to insert the current datetime, if it is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Date doesn't really matter. We're not displaying it. You could probably remove it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was not sure how much more you are planning to add, so I just left a couple of comments rather than a full review
date: 2017-09-07T20:10:05-07:00 | ||
draft: false | ||
weight: 50 | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a starting point, you could just copy and paste the contributing.md that we have in the root
@erwinvaneyk - yes still WIP and will add another batch tomorrow for review. |
Ok understood, @vishal-biyani :) It might be an idea to have a "wip" label for these kinds of PRs |
@@ -0,0 +1,6 @@ | |||
--- | |||
title: "Watch" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggest removing this; a watch isn't really a separate concept, but just a type of trigger.
@@ -1,181 +1,6 @@ | |||
--- | |||
title: "Installation Guide" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's wrong with "guide"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am sorry, I have too many coding tendencies while also typing normal text :) and I like camel case 😈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I didn't mean the casing of the word guide/Guide. I meant, the diff removes the word guide and changes "Installation Guide" to just "Installation", and I was wondering why you did that.
date: 2017-09-07T20:10:05-07:00 | ||
draft: false | ||
weight: 20 | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I think I disagree with this idea... I really dislike jumping around 3 different pages while trying to install something. I feel like the all-in-one page is better for new users. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On second thoughts, I agree. It is not friendly to have jump n number of pages to install something. Will put in a single one.
date: 2017-09-07T20:10:05-07:00 | ||
draft: false | ||
weight: 26 | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kinda vague... remove unless we have something to populate this with
@@ -1,7 +1,8 @@ | |||
--- | |||
title: "Kubernetes Quick Install" | |||
title: "Quick Installation" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this is actually a Kubernetes install, not Fission install
@@ -0,0 +1,6 @@ | |||
--- | |||
title: "Environemnt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo (spelling)
@@ -0,0 +1,6 @@ | |||
--- | |||
title: "Executor" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't really write a usage guide to the executor; it's a component of our implementation. We should write a usage guide to controlling function execution, latency, throughput, memory costs etc.
@@ -0,0 +1,6 @@ | |||
--- | |||
title: "Watch" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment as above, watches are just a type of trigger
@@ -0,0 +1,25 @@ | |||
--- | |||
title: "Environemnt" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo
@@ -0,0 +1,6 @@ | |||
--- | |||
title: "Executor" | |||
date: 2017-09-07T20:10:05-07:00 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Date doesn't really matter. We're not displaying it. You could probably remove it.
|
||
# Fission Concepts | ||
|
||
#### Understanding Fission terminology and concepts |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be a link, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Python 3 | `fission/python-env` | | ||
| Ruby | `fission/ruby-env` | | ||
|
||
To create custom environments you can extend above or create your own environment from scratch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can extend one of the environments in the list, or create...
|
||
# Contributing to Fission | ||
|
||
### Development guide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
link
|
||
### Installing and upgrading Fission |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
link
|
||
# Using Fission | ||
|
||
### Examples, code samples and How To's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where does this show up? Also, suggest rephrasing to "Usage guides, tutorials and examples".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -1,8 +1,11 @@ | |||
--- | |||
title: "Access secret/configmap in function" | |||
title: "Secret/configmap in function" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I liked the previous title better (though "Accessing secrets..." is better than "Access secret...")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
### Create an environment | ||
|
||
You can create an environment from node-env image and specify the resources such as memory and CPU along with poolsize. The poolsize controls how many warm pods are created initially. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"You can create an environment on your cluster from an image for that language. Optionally, you can specify CPU and memory resource limits. You can also specify the number of initially pre-warmed pods, which is called the poolsize."
When you create an environment, you can specify a builder image and builder command which will be used for building from source code. You can override the build command when creating a function. For more details on builder and packages you should check out examples in [Functions](../functions) and [packages](../package) | ||
|
||
``` | ||
fission env create --name python --image gcr.io/fission-ci/python-env:test --builder gcr.io/fission-ci/python-env-builder:test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't be telling people to use our test builds... this guide should point at released images only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I had not found official builder image when I wrote this, fixed now.
weight: 43 | ||
--- | ||
|
||
### Pool executor, low latency |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this usage guide we shouldn't talk about the executor in the subheadings, only the use case. E.g.
"Functions with no cold start overhead"
"Functions with no idle cost"
"Autoscaling"
@@ -0,0 +1,130 @@ | |||
--- | |||
title: "Package" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a better title
Also addresses #470 |
|
||
Newdeploy executortype can be used for requests with no particular low-latency requirements, such as those invoked asynchronously, minscale can be set to zero. In this case the Kubernetes deployment and other objects will be created on first invocation of the function. Subsequent requests can be served by the same deployment. If there are no requests for certain duration then the idle objects are cleaned up. This mechanism ensures resource consumption only on demand and is a good fit for asynchronous requests. | ||
|
||
For requests where latency requirements are stringent, a minscale greater than zero can beset. This essentially keeps a minscale number of pods ready when you create a function. When the function is invoked, there is no delay since the pod does not have to created. Also minscale ensures that the pods are not cleaned up even if the function is idle. This is great for functions where lower latency is more important than saving resource consumption when functions are idle. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a minscale greater than zero can be set
Review status: 0 of 25 files reviewed at latest revision, 27 unresolved discussions. Documentation/docs-site/content/concepts/executor.en.md, line 25 at r8 (raw file): Previously, life1347 (Ta-Ching Chen) wrote…
there is no delay since the pod does not have Documentation/docs-site/content/concepts/executor.en.md, line 29 at r9 (raw file):
In future, a more intelligent dispatch mechanism ... Documentation/docs-site/content/concepts/executor.en.md, line 39 at r9 (raw file):
based on CPU usage ... Documentation/docs-site/content/concepts/functions.en.md, line 6 at r9 (raw file):
is a piece of... Documentation/docs-site/content/concepts/trigger.en.md, line 15 at r9 (raw file):
Time triggers follow Documentation/docs-site/content/concepts/trigger.en.md, line 17 at r9 (raw file):
Time trigger based ... Documentation/docs-site/content/concepts/trigger.en.md, line 21 at r9 (raw file):
Comments from Reviewable |
Review status: 0 of 25 files reviewed at latest revision, 27 unresolved discussions. Documentation/docs-site/content/concepts/_index.en.md, line 10 at r5 (raw file): Previously, vishal-biyani (Vishal) wrote…
Done. Documentation/docs-site/content/concepts/environments.en.md, line 2 at r1 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/concepts/environments.en.md, line 24 at r5 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/concepts/executor.en.md, line 3 at r1 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/concepts/executor.en.md, line 25 at r8 (raw file): Previously, life1347 (Ta-Ching Chen) wrote…
Done. Documentation/docs-site/content/concepts/executor.en.md, line 29 at r9 (raw file): Previously, life1347 (Ta-Ching Chen) wrote…
Done. Documentation/docs-site/content/concepts/executor.en.md, line 39 at r9 (raw file): Previously, life1347 (Ta-Ching Chen) wrote…
Done. Documentation/docs-site/content/concepts/functions.en.md, line 6 at r9 (raw file): Previously, life1347 (Ta-Ching Chen) wrote…
Done. Documentation/docs-site/content/concepts/trigger.en.md, line 15 at r9 (raw file): Previously, life1347 (Ta-Ching Chen) wrote…
Done. Documentation/docs-site/content/concepts/trigger.en.md, line 17 at r9 (raw file): Previously, life1347 (Ta-Ching Chen) wrote…
Done. Documentation/docs-site/content/concepts/trigger.en.md, line 21 at r9 (raw file): Previously, life1347 (Ta-Ching Chen) wrote…
Done. Documentation/docs-site/content/contributing/_index.en.md, line 6 at r1 (raw file): Previously, erwinvaneyk (Erwin van Eyk) wrote…
Done. Documentation/docs-site/content/contributing/_index.en.md, line 10 at r5 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/installation/_index.en.md, line 10 at r5 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/installation/kubernetessetup.en.md, line 2 at r1 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/usage/_index.en.md, line 10 at r5 (raw file): Done. Documentation/docs-site/content/usage/access-secret-cfgmap-in-function.md, line 2 at r5 (raw file): Previously, vishal-biyani (Vishal) wrote…
Done. Documentation/docs-site/content/usage/environments.en.md, line 2 at r1 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/usage/environments.en.md, line 9 at r5 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/usage/environments.en.md, line 22 at r5 (raw file): Previously, vishal-biyani (Vishal) wrote…
Done. Documentation/docs-site/content/usage/executor.en.md, line 2 at r1 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/usage/executor.en.md, line 7 at r5 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/usage/package.en.md, line 2 at r5 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/concepts/watch.en.md, line 2 at r1 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/installation/known-issues.en.md, line 6 at r1 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/resources/_index.en.md, line 1 at r1 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Documentation/docs-site/content/usage/watch.en.md, line 2 at r1 (raw file): Previously, soamvasani (Soam Vasani) wrote…
Done. Comments from Reviewable |
Just a few comments, then good to merge -- Reviewed 7 of 39 files at r1, 4 of 17 files at r3, 4 of 22 files at r4, 7 of 11 files at r5, 7 of 8 files at r6, 5 of 6 files at r7, 3 of 3 files at r8, 3 of 3 files at r10. Documentation/docs-site/content/concepts/package.en.md, line 7 at r10 (raw file):
This doc needs to specifically define both archives and packages. But, later. Let's merge this and improve it iteratively. Documentation/docs-site/content/usage/access-secret-cfgmap-in-function.md, line 6 at r10 (raw file):
Can you please remove this comment and file an issue listing out the specific things in this doc that you think should be changed? Documentation/docs-site/content/usage/trigger.en.md, line 2 at r10 (raw file):
This is missing K8s watch triggers. (Add it later, no need to block this) Comments from Reviewable |
Updated #499 for changes to cover later and addressed one comment. |
Adding documentation as per new structure and details
This change is