tiltfile: add only parameter to docker_build#1711
Conversation
| f.dockerfile("Dockerfile") | ||
| f.yaml("foo.yaml", deployment("foo", image("gcr.io/foo"))) | ||
| f.file("Tiltfile", ` | ||
| docker_build('gcr.io/foo', '.', ignore="**/*.md", only=["common/**", "myservice/**"]) |
There was a problem hiding this comment.
Are there any other weird interactions I should add tests for here?
There was a problem hiding this comment.
There are probably a bunch. It would be good to say in the docs that only is built on dockerignore, and so may have issues.
What if you put the same file in both ignore and only?
What if you have elements that start with ! in only?
What if you have \n in strings?
I'm not sure it's worth trying to catch all of these right now, vs. being clear that if you need to be extremely dlever you should talk to us.
There was a problem hiding this comment.
Oooh these are great. I'm going to add some tests for them, and make myself a TODO to write up some docs about precedence rules and such.
There was a problem hiding this comment.
For things that don't block this epic, but we should do, it can be better to leave it as a TODO in code than in your notebook so that if you end up working on something else, your thoughts can help the next person to take it up.
dbentley
left a comment
dbentley
left a comment
There was a problem hiding this comment.
This seems to match the filter that's created, but not the end-to-end "this Tiltfile plus an edit to this file does/does not end up in the context"
|
@dbentley that's true and I think that would be the job of a test in the build/container test suite. My instinct would be to put that in a separate PR. |
|
Fine with another PR; it seems like important testing to have some version of before closing this epic |
|
The build/container wouldn't include the Tiltfile piece, right? f.tiltfile("docker_build('foo', '.', only=['foo'])") |
dbentley
left a comment
dbentley
left a comment
There was a problem hiding this comment.
As discussed aloud, I was missing that these tests already include evaluating the matcher against a path.
2 participants
No description provided.