-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Documentation updates, mainly for Python API #5531
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
Conversation
Jennifer88huang-zz
left a comment
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.
@candlerb Thank you very much for your feedback and contributions.
We should have removed the deprecated files functions-api.md and functions-state.md files from the master.
We've adopted the new structure for Pulsar Functions in #4554. It's not complete at the moment. If you have any issue on the new structure, feel free to let us know.
site2/docs/functions-develop.md
Outdated
| Note that functions can be written in python2 or python3, but pulsar | ||
| currently only looks for "python" as the interpreter to execute them. | ||
|
|
||
| A recent Ubuntu system may have only "python3" but not "python", in which | ||
| case functions will fail to start. As a workaround you can create a symlink, but beware this has some | ||
| [risks](https://askubuntu.com/questions/320996/how-to-make-python-program-command-execute-python-3#answer-475815): | ||
|
|
||
| ```bash | ||
| sudo update-alternatives --install /usr/bin/python python /usr/bin/python3 10 | ||
| ``` | ||
|
|
||
| If you choose to do this, be careful not to install any other package which | ||
| depends on "python" (2.x) |
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.
Note
You can write Pulsar Functions in python2 or python3. However, Pulsar only looks forpythonas the interpreter.
If you're running Pulsar Functions on Ubuntu system that only supports
python3, you might fail to
start the functions. In this case, you can create a symlink. However, such action
has potential risks.
sudo update-alternatives --install /usr/bin/python python /usr/bin/python3 10If you create a symlink, you'd better not install any other package that depends on "python" (2.x).
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 tiny suggestion on this: If we can summarize the risks briefly, we can use a brief summary, and not use external links.
Reason: The external link might change or someone might delete the Q&A in this link, then the link in our docs will be broken, it's hard to maintain external links.
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.
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, sure.
site2/docs/functions-develop.md
Outdated
|
|
||
| Since Pulsar 2.1.0 release, Pulsar integrates with Apache BookKeeper [table service](https://docs.google.com/document/d/155xAwWv5IdOitHh1NVMEwCMGgB28M3FyMiQSxEpjE-Y/edit#heading=h.56rbh52koe3f) to store the `State` for functions. For example, a `WordCount` function can store its `counters` state into BookKeeper table service via Pulsar Functions State API. | ||
|
|
||
| States are key-value pairs, where the key is a string and the value is arbitrary binary data - counters are stored as 64-bit big-endian binary values. Keys are scoped to an individual pulsar function, but shared between all instances of that 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.
| States are key-value pairs, where the key is a string and the value is arbitrary binary data - counters are stored as 64-bit big-endian binary values. Keys are scoped to an individual pulsar function, but shared between all instances of that function. | |
| States are key-value pairs, where the key is a string and the value is arbitrary binary data - counters are stored as 64-bit big-endian binary values. Thought keys are scoped to an individual Pulsar Function, the keys are shared among all instances of that 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.
s/Thought/Though/ ?
I think the conjunction isn't useful here, as it implies the user already knows the scoping of keys. I don't think it's mentioned earlier, and without this knowledge I might have guessed they were scoped differently (to the pulsar "namespace" that the function executes within, for example)
So as a user, I just want a statement which answers the question: "What's the scope of the key?"
I would be happy with "Keys are scoped to the pulsar function". I thought it worth clarifying functions versus function instances, but maybe that's unnecessary. State storage which wasn't shared between function instances wouldn't be very useful.
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.
"Though", sorry for the typo.
If you think the conjunction is useful, you can remove it.
site2/docs/functions-state.md
Outdated
|
|
||
| ## API | ||
|
|
||
| <!--DOCUSAURUS_CODE_TABS--> |
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.
Thank you very much for adding those valuable info. Could we add those info in the [functions-develop.md#state-storage] section?
Do you think I should wait until the structure is finalized before reworking this patch? |
You do not need to wait, just go ahead with your patch. |
7228510 to
f4d954d
Compare
|
I have force-pushed new version; this also removes functions-api.md and functions-state.md |
|
Never mind, I will open new PR |
Motivation
Documentation improvements
Modifications
--pyand--classnameNote:
functions-stateandfunctions-apiare currently unlinked from the navbar. Need to decide what to do about these - e.g. move valuable parts into "functions-develop" and then "git rm" them.