Skip to content

[BEAM-3951] Migrate container instructions and builds to Gradle#4958

Merged
lukecwik merged 3 commits intoapache:masterfrom
herohde:flinkbug
Mar 28, 2018
Merged

[BEAM-3951] Migrate container instructions and builds to Gradle#4958
lukecwik merged 3 commits intoapache:masterfrom
herohde:flinkbug

Conversation

@herohde
Copy link
Copy Markdown
Contributor

@herohde herohde commented Mar 27, 2018

  • Also move out Go examples for build performance

 * Also move out Go examples for build performance
@herohde
Copy link
Copy Markdown
Contributor Author

herohde commented Mar 27, 2018

R: @lukecwik

@herohde herohde changed the title Migrate container instructions and builds to Gradle [BEAM-3951] Migrate container instructions and builds to Gradle Mar 27, 2018
Copy link
Copy Markdown
Member

@lukecwik lukecwik left a comment

Choose a reason for hiding this comment

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

Minor changes.

}
}

def containerImageName(name) {
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.

Add method comment.

Interesting that def works, before I was sticking everything onto ext. This seems much cleaner. Consider swapping all usages of ext.fooBar = {} to def fooBar

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added. I also stick it into ext below.


Similarly for the Java SDK harness container image. If you want to push the same image
Similarly for the Java and Go SDK harness container images. If you want to push the same image
to multiple registries, you can retagging the image using `docker tag` and push.
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.

retagging -> retag

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

}
name "${repositoryRoot}/go:latest"
name containerImageName("go")
files "./build/"
Copy link
Copy Markdown
Member

@lukecwik lukecwik Mar 27, 2018

Choose a reason for hiding this comment

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

Note that with this setup we are building the boot binary multiple times, once per container. Not a blocking issue, just an observation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not sure I understand what you mean. We are only building one container image, like before.

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.

We have the same golang definition to build the boot.go binary in sdks/java/container, sdks/go/container, and sdks/python/container.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They are different boot.go files.

@herohde
Copy link
Copy Markdown
Contributor Author

herohde commented Mar 27, 2018

Thanks @lukecwik. PTAL

@herohde
Copy link
Copy Markdown
Contributor Author

herohde commented Mar 28, 2018

Updated the helper function. @lukecwik PTAL

@lukecwik lukecwik merged commit 25e4a5b into apache:master Mar 28, 2018
@herohde herohde deleted the flinkbug branch March 28, 2018 04:44
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.

2 participants