-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Attach build to docker daemon functionality to gradle task #265
Conversation
private static final String CACHE_DIRECTORY_NAME = "jib-cache"; | ||
|
||
/** {@code User-Agent} header suffix to send to the registry. */ | ||
private static final String USER_AGENT_SUFFIX = "jib-gradle-plugin"; |
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 isn't necessary anymore since it's not using a registry.
import org.gradle.api.tasks.TaskAction; | ||
|
||
/** Builds a container image and exports to the default Docker daemon. */ | ||
public class BuildDockerTask extends DefaultTask { | ||
|
||
/** Directory name for the cache. The directory will be relative to the build output directory. */ | ||
private static final String CACHE_DIRECTORY_NAME = "jib-cache"; |
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.
We'd probably want this to be shared and the same with BuildImageTask
. Maybe ProjectProperties
would be a good place for it, and the tasks would call like ProjectProperties#getCacheDirectory
.
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.
Do you want me to do this now or hold off on this until last bullet of #262?
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.
Can hold off but maybe add a TODO here or at the top of the class
|
||
/** Converts an {@link ImageConfiguration} to an {@link Authorization}. */ | ||
@Nullable | ||
private static Authorization getImageAuthorization(ImageConfiguration imageConfiguration) { |
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.
We could prob just put this in ImageConfiguration
.
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 question about #262
.setJvmFlags(jibExtension.getJvmFlags()) | ||
.build(); | ||
|
||
// Disables annoying Apache HTTP client logging. |
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.
These HTTP stuff can be removed.
Part of #243.
There's a lot of duplicate code here, so this introduces a few refactoring TODOs, which I grouped into issue #262.