Skip to content

[BEAM-1218] Removing some of the dataflow references.#1774

Closed
aaltay wants to merge 2 commits intoapache:python-sdkfrom
aaltay:textc
Closed

[BEAM-1218] Removing some of the dataflow references.#1774
aaltay wants to merge 2 commits intoapache:python-sdkfrom
aaltay:textc

Conversation

@aaltay
Copy link
Copy Markdown
Member

@aaltay aaltay commented Jan 13, 2017

No description provided.

@aaltay
Copy link
Copy Markdown
Member Author

aaltay commented Jan 13, 2017

R: @davorbonaci Please review.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

@aaltay aaltay closed this Jan 13, 2017
@aaltay aaltay reopened this Jan 13, 2017
@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6542/
--none--

Copy link
Copy Markdown
Member

@davorbonaci davorbonaci left a comment

Choose a reason for hiding this comment

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

Looks good; left a few comments.

# limitations under the License.
#

"""Dataflow credentials and authentication."""
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All, or at least parts of this, might be Dataflow / GCP specific.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Correct. Reverted this for now. I will tackle along with the future cleanups.

Map transform will get on each call *one* row of the main table and *all* rows
of the side table. The execution framework may use some caching techniques to
share the side inputs between calls in order to avoid excessive reading::
(common case) is expected to be massive and the execution framework will make
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you are talking about what execution engine will do?
if you are using native source, it is runner-specific.
if not, then it is the source guaranteeing that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Rewrote this part to be more generic, and easier for the user to understand.

@aaltay
Copy link
Copy Markdown
Member Author

aaltay commented Jan 13, 2017

Thank you @davorbonaci please take another look.

@asfbot
Copy link
Copy Markdown

asfbot commented Jan 13, 2017

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6547/
--none--

@davorbonaci
Copy link
Copy Markdown
Member

LGTM. Merged.

asfgit pushed a commit that referenced this pull request Jan 13, 2017
@aaltay
Copy link
Copy Markdown
Member Author

aaltay commented Jan 14, 2017

Thank you.

@aaltay aaltay closed this Jan 14, 2017
@aaltay aaltay deleted the textc branch January 27, 2017 22:39
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.

3 participants