mostly incremental builds #817
Conversation
| @@ -1,26 +1,2 @@ | |||
| """ | |||
There was a problem hiding this comment.
sorry about these, need to rerun formatting.
airbyte-integrations/connectors/source-exchangeratesapi-singer/build.gradle
Show resolved
Hide resolved
airbyte-integrations/connectors/source-marketo-singer/build.gradle
Outdated
Show resolved
Hide resolved
airbyte-integrations/connectors/source-salesforce-singer/setup.py
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,11 @@ | |||
| plugins { | |||
| def result = [] as Set<String> | ||
|
|
||
| dockerfile.eachLine { line -> | ||
| if (line.startsWith("FROM ")) { |
There was a problem hiding this comment.
What is the worse that can happen if the parsing doesn't work? This piece seems a bit brittle (not saying we should change it but we need to understand the limits and how it can fail, and fail loudly if it is outside of the limit)
There was a problem hiding this comment.
If there's a way it's depending on a separate image that isn't caught by COPY --from or FROM, that's the worst case, because it won't be caught at all.
If it does use COPY --from or FROM but uses some unexpected syntax, it'd end up adding the wrong string or an empty string which should almost always result in an inability to find a current image hash, which would always require image rebuilding (slow but not too terrible of an outcome).
I can at least change this to assert the the image is not empty.
| exceptionFormat "full" | ||
| } | ||
|
|
||
| if(project.hasProperty("airbyteDocker")) { |
There was a problem hiding this comment.
shouldn't this dependOn belong in the airbyte-docker?
There was a problem hiding this comment.
I don't think so. I actually just did something like this for the source test too, since airbyte-docker should be applied first, before airbyte-integration-test-java exists.
|
AMAZING PR! |
…tiontest is applied
| rootDir = project.rootProject.rootDir | ||
| projectDir = project.projectDir | ||
| projectFiles = filteredProjectFiles | ||
| idFileOutput = project.file(Paths.get(project.rootProject.rootDir.absolutePath, '.dockerversions', hash).toString()) |
There was a problem hiding this comment.
i think i'd prefer not to remove it on a clean since there is already a diff computation mechanism, so this LGTM
This introduces more cachability and uses plugins to configure all of our behavior.
More is cacheable than before. This also uses buildkit for docker builds which should be faster. Incremental isn't fully working for Python since I didn't figure out how to get the internal
pipInstallto be cacheable.Recommended order:
buildSrc/*