Improve error message for empty containers and tasks #11
Conversation
…iendly message if it does not
|
Awesome, thanks @pameck, I'll take a look now. |
| @@ -39,7 +51,6 @@ data class ConfigurationFile( | |||
| if (pathResolver.relativeTo.root == pathResolver.relativeTo) { | |||
| throw ConfigurationException("No project name has been given explicitly, but the configuration file is in the root directory and so a project name cannot be inferred.") | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
Can you please revert this whitespace change?
| @@ -56,6 +56,7 @@ class ConfigurationLoader( | |||
| } | |||
|
|
|||
| Files.newInputStream(path, StandardOpenOption.READ).use { | |||
|
|
|||
There was a problem hiding this comment.
Can you please revert this whitespace change?
| } | ||
|
|
||
| private fun fromFileToContainerConfiguration(containerName: String, fileContainerConfig: ContainerFromFile, pathResolver: PathResolver): Container { | ||
| if (fileContainerConfig == null) { |
There was a problem hiding this comment.
This is awful behaviour from Jackson... it really shouldn't be providing a null value here for something that can't be null. Not much we can do about that though, and it's already been reported at FasterXML/jackson-module-kotlin#144. (This makes me wonder where else we need to cover this case...)
It also means the compiler gives a warning about this condition (https://travis-ci.org/charleskorn/batect/builds/398475902#L577), because it should theoretically never be true. Could you perhaps make fileContainerConfig nullable (ie. ContainerFromFile? rather than ContainerFromFile) so that the compiler warning goes away?
|
@charleskorn thanks for the Kotlin tip on the nullable stuff, all fixed and the blank spaces reverted. Don't merge it just yet, I'll add the check for tasks too on this PR. |
…y message if it does not
|
@charleskorn re-review. I added the check for empty tasks as well. |
charleskorn
changed the title
|
Thanks @pameck, looks perfect. |
…ith an image that uses a non-default syntax. The output previously looked something like (note output for #3 and #4): #1 [internal] load remote build context #1 DONE #2 copy /context / #2 DONE #3 resolve image config for docker.io/docker/dockerfile:1.1-experimental #3 ... #4 docker-image://docker.io/docker/dockerfile:1.1-experimental@sha256:de85b2f3a3e8a2f7fe48e8e84a65f6fdd5cd5183afa6412fff9caa6871649c44 #4 ... #5 [internal] load metadata for docker.io/library/ruby:2.7.2 #5 DONE #6 [ 1/12] FROM docker.io/library/ruby:2.7.2 #6 DONE #7 [ 2/12] RUN mkdir -p /tools #7 CACHED #8 [ 3/12] COPY health-check.sh /tools #8 CACHED #9 [ 4/12] RUN mkdir -p /app #9 CACHED #10 [ 5/12] COPY app/Gemfile app/Gemfile.lock /app/ #10 CACHED #11 [ 6/12] WORKDIR /app #11 CACHED #12 [ 7/12] RUN bundle config set deployment true #12 CACHED #13 [ 8/12] RUN bundle config set without development #13 CACHED #14 [ 9/12] RUN bundle install #14 CACHED #15 [10/12] COPY app/config.ru /app #15 CACHED #16 [11/12] COPY app/bin /app/bin #16 CACHED #17 [12/12] COPY app/lib /app/lib #17 ... #3 resolve image config for docker.io/docker/dockerfile:1.1-experimental #3 DONE #4 docker-image://docker.io/docker/dockerfile:1.1-experimental@sha256:de85b2f3a3e8a2f7fe48e8e84a65f6fdd5cd5183afa6412fff9caa6871649c44 #4 CACHED #17 [12/12] COPY app/lib /app/lib #17 CACHED #18 exporting to image #18 exporting layers: done #18 writing image sha256:ace2572ab366a9153b23ccc0667cc6163a32a3cbb96bfef237a25c68005699a6: done #18 naming to docker.io/library/batect-sample-ruby-international-transfers-service: done #18 DONE
Fixes #4
It was assumed the containers were always defined in the config file, but when only the name of the container is defined, Jackson converts it to an entry with null as the value.
This PR checks whether the container exists before transforming it to prevent the
NullPointerExceptionand produce a more user friendly error message.