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
[BEAM-1665] Add Pipeline I/O section to website - outline + move some existing txt #173
Conversation
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): Jenkins built the site at commit id b0e0862 with Jekyll and staged it here. Happy reviewing. Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again. |
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.
Sorry for the delay, thanks for setting up the skeleton for a new I/O section! I've added some comments.
--- | ||
|
||
[Pipeline I/O Table of Contents]({{site.baseurl}}/documentation/io/io-toc/) | ||
|
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 page is missing a title
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.
done, thanks!
src/documentation/io/built-in.md
Outdated
|
||
[Pipeline I/O Table of Contents]({{site.baseurl}}/documentation/io/io-toc/) | ||
|
||
This is the set of I/O transforms currently available in Beam. |
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, what about something like "This table contains the currently available I/O transforms."
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.
done
src/documentation/io/built-in.md
Outdated
|
||
This is the set of I/O transforms currently available in Beam. | ||
|
||
Consult the [Programming Guide I/O section]({{site.baseurl }}/documentation/programming-guide#io) for general usage instructions and the javadoc/pydoc for the particular I/O transforms. |
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 initially read it quickly as the instructions and javadoc/pydoc are both available in the programming guide. perhaps add a comma and "see" --> "usage instructions, and see the javadoc ..."
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.
done!
src/documentation/io/contributing.md
Outdated
|
||
# Contributing I/O Transforms to Beam | ||
|
||
* If you are planning to contribute your I/O transform to the beam community, you'll be going through the normal beam contribution life cycle - see the Apache Beam Contribution Guide for more details. |
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 general we've been trying to always have first mention of Beam be prefixed with Apache. maybe add Apache to the H1 title?
- beam should be capitalized (x2)
- link to the contribution 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.
done. I ended up from the "to Beam" part, but I moved the Apache to the first instance of beam in the doc
src/documentation/io/testing.md
Outdated
|
||
# Next steps | ||
|
||
If you've got a well tested I/O transform, why not contribute it to Beam? Read all about 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.
"If you have a"
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.
done
</tr> | ||
</table> | ||
### Beam-provided I/O Transforms | ||
There is a page devoted to [Beam-provided I/O Transforms]({{site.baseurl }}/documentation/io/built-in/). |
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'm wondering if it might be good to specify what's roughly on the page, so users can easily decide if they want/need to jump there or not. though, this would need to be updated again later, after/if more is added to the page. and admittedly, the sentence below is a bit redundant right now while it's just a list of the provided transforms.
See the Beam-provided I/O Transforms page for a list of the currently available I/O transforms.
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.
yeah, I struggled with the wording here, this feels better.
done.
src/documentation/sdks/java.md
Outdated
|
||
|
||
## Pipeline I/O | ||
There is a page devoted to [Beam-provided I/O Transforms]({{site.baseurl }}/documentation/io/built-in/). |
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 as above
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.
done.
title: "Authoring I/O - Overview" | ||
permalink: /documentation/io/authoring-overview/ | ||
--- | ||
|
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.
nit: a couple extra blank lines
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.
done
|
||
[Pipeline I/O Table of Contents]({{site.baseurl}}/documentation/io/io-toc/) | ||
|
||
# Authoring I/O - Overview |
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.
the two titles (metadata at top and this H1) are missing the "Transforms", "Authoring I/O Transforms"
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.
done
|
||
# Authoring I/O - Overview | ||
|
||
_A guide for those who need to connect to a data store not covered by a [Built-in I/O Transforms]({{site.baseurl }}/documentation/io/built-in/)_ |
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.
slight tweaks, maybe:
A guide for users who need to connect to a data store that isn't supported by the [Built-in I/O Transforms].
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.
done
…ntent * I did not to go with a single page for all this content b/c both java and python have enough unique content that they deserve their own separate sections (ie, just tabs on the code isn't enough), and the "click to the next page" model currently implemented allows the user to pick java vs python, but then after reading those pages, the next page for both points at the same place - the users mostly follow the same path, but for java vs python specific content, they will diverge then converge again. * I moved the "list of built-in I/O" content over to it's own separate page since it'd be nice to have more content there - e.g. capabilities matrix, and it felt special enough to pull out of the programming guide. * We decided not to put all of this content in the contribute section of the site since the expectation is we don't think all users will contribute their IO transforms, so we want most of the docs to just be about writing an IO transforms, and they lay out the expectations in the contribute part of the IO section.
Refer to this link for build results (access rights to CI server 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.
okay! thanks so much for all your comments.
--- | ||
|
||
[Pipeline I/O Table of Contents]({{site.baseurl}}/documentation/io/io-toc/) | ||
|
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.
done, thanks!
title: "Authoring I/O - Overview" | ||
permalink: /documentation/io/authoring-overview/ | ||
--- | ||
|
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.
done
|
||
[Pipeline I/O Table of Contents]({{site.baseurl}}/documentation/io/io-toc/) | ||
|
||
# Authoring I/O - Overview |
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.
done
|
||
# Authoring I/O - Overview | ||
|
||
_A guide for those who need to connect to a data store not covered by a [Built-in I/O Transforms]({{site.baseurl }}/documentation/io/built-in/)_ |
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.
done
src/documentation/io/built-in.md
Outdated
|
||
[Pipeline I/O Table of Contents]({{site.baseurl}}/documentation/io/io-toc/) | ||
|
||
This is the set of I/O transforms currently available in Beam. |
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.
done
src/documentation/io/built-in.md
Outdated
|
||
This is the set of I/O transforms currently available in Beam. | ||
|
||
Consult the [Programming Guide I/O section]({{site.baseurl }}/documentation/programming-guide#io) for general usage instructions and the javadoc/pydoc for the particular I/O transforms. |
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.
done!
src/documentation/io/contributing.md
Outdated
|
||
# Contributing I/O Transforms to Beam | ||
|
||
* If you are planning to contribute your I/O transform to the beam community, you'll be going through the normal beam contribution life cycle - see the Apache Beam Contribution Guide for more details. |
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.
done. I ended up from the "to Beam" part, but I moved the Apache to the first instance of beam in the doc
src/documentation/io/testing.md
Outdated
|
||
# Next steps | ||
|
||
If you've got a well tested I/O transform, why not contribute it to Beam? Read all about 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.
done
</tr> | ||
</table> | ||
### Beam-provided I/O Transforms | ||
There is a page devoted to [Beam-provided I/O Transforms]({{site.baseurl }}/documentation/io/built-in/). |
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.
yeah, I struggled with the wording here, this feels better.
done.
src/documentation/sdks/java.md
Outdated
|
||
|
||
## Pipeline I/O | ||
There is a page devoted to [Beam-provided I/O Transforms]({{site.baseurl }}/documentation/io/built-in/). |
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.
done.
Refer to this link for build results (access rights to CI server needed): Jenkins built the site at commit id a6f367a with Jekyll and staged it here. Happy reviewing. Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again. |
Thanks, LGTM as a skeleton starting point. Looking forward to seeing it full of good stuff. |
Thanks for the review Melissa! |
I'm worried about publishing this as-is. We could do it, but users will just see a ton of TODOs, probably resulting in worse experience than without this content. @ssisk, can we perhaps merge it, but comment it out, so users don't see so many TODOs? |
sure, I'll take a pass and see what I can comment out so that it doesn't present a ton of TODOs to the users. That'll probably look like: initially the TOC page will be there and the "using" section will be visible, but not the authoring section. |
Refer to this link for build results (access rights to CI server needed): |
okay! I've:
PTAL |
Refer to this link for build results (access rights to CI server needed): Jenkins built the site at commit id 1f4d996 with Jekyll and staged it here. Happy reviewing. Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again. |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): Jenkins built the site at commit id be4463f with Jekyll and staged it here. Happy reviewing. Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again. |
Refer to this link for build results (access rights to CI server needed): |
Refer to this link for build results (access rights to CI server needed): Jenkins built the site at commit id 76bf199 with Jekyll and staged it here. Happy reviewing. Note that any previous site has been deleted. This staged site will be automatically deleted after its TTL expires. Push any commit to the pull request branch or re-trigger the build to get it staged again. |
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.
LGTM.
Test breakage seems unrelated GitHub outage. Merging.
This PR has a fair number of TODOs in the website content, but I have PRs queued up with more content from the IO authoring guide and none of the TODOs are for content already on the website.
It has very little actual new content - mostly, it created structure and moves existing content around.
R @melap