Skip to content

[BEAM-2963] Stage the portable pipeline; put URL in pipeline options#4089

Merged
asfgit merged 1 commit into
apache:masterfrom
kennknowles:stage-pipeline-with-options
Nov 9, 2017
Merged

[BEAM-2963] Stage the portable pipeline; put URL in pipeline options#4089
asfgit merged 1 commit into
apache:masterfrom
kennknowles:stage-pipeline-with-options

Conversation

@kennknowles
Copy link
Copy Markdown
Member

@kennknowles kennknowles commented Nov 7, 2017

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean verify to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

@kennknowles kennknowles changed the title [BEAM-2884] Stage the portable pipeline; put URL in pipeline options [BEAM-2963] Stage the portable pipeline; put URL in pipeline options Nov 7, 2017
@kennknowles
Copy link
Copy Markdown
Member Author

R: @lukecwik

Same story, new place to put the info.

@kennknowles kennknowles force-pushed the stage-pipeline-with-options branch from 8356123 to 6c1ebdb Compare November 7, 2017 05:37
@lukecwik
Copy link
Copy Markdown
Member

lukecwik commented Nov 8, 2017

To my knowledge, arbitrary pipeline options don't make it to the runner harness.
Why the swap away from worker metadata configuration, did using a map -> list of kv conversion not work?

@kennknowles
Copy link
Copy Markdown
Member Author

The metadata field is a Map. That wouldn't typecheck.

@kennknowles
Copy link
Copy Markdown
Member Author

You mean hacking it with a custom jackson hook? I'd rather not mess with that, since pipelineoptions is already a pretty arbitrary bag and actually sort of better than VM metadata for this kind of data.

@lukecwik
Copy link
Copy Markdown
Member

lukecwik commented Nov 8, 2017

Arbitrary pipeline options still don't make it to the runner harness.

@kennknowles
Copy link
Copy Markdown
Member Author

@herohde can you comment on this? I think that we have adequate control to get this info to the right place noninvasively.

@herohde
Copy link
Copy Markdown
Contributor

herohde commented Nov 8, 2017

Pipeline options are available to the runner harness in Dataflow, so this approach would work fine. It's trivial to make the runner harness grab what it needs here.

@herohde
Copy link
Copy Markdown
Contributor

herohde commented Nov 8, 2017

LGTM

1 similar comment
@lukecwik
Copy link
Copy Markdown
Member

lukecwik commented Nov 9, 2017

LGTM

@asfgit asfgit merged commit 6c1ebdb into apache:master Nov 9, 2017
asfgit pushed a commit that referenced this pull request Nov 9, 2017
@kennknowles kennknowles deleted the stage-pipeline-with-options branch April 24, 2018 21:20
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.

4 participants