Two small improvements #84
Conversation
ensonic
commented
Dec 1, 2017
ensonic
commented
- mentions dependencies in the docs
- make the namespace attribute on k8s_object optional
|
Can one of the admins verify this patch? |
README.md
Outdated
| ## Dependencies | ||
|
|
||
| The rules will require the `kubectl` and `gcloud` tools when executing the `run` | ||
| action from bazel. |
There was a problem hiding this comment.
Technically gcloud isn't required unless you are using GCP.
|
Test this please |
|
Thanks for the PR @ensonic One small nit, but otherwise looks good. I'll merge it into a branch and then open a PR to merge things from there so our integration testing will run since Travis doesn't provide secrets unless the PR is from a branch on the main repo. |
|
I'll update the doc patch. I run the tests, but this is probably not covered. I'll take another look and see if I can add a test for it. |
|
Sorry, that comment was for Bazel CI (this is covered by Travis already) :) |
|
Ping? |
|
Sorry I didn't realize you'd pushed more commits. If you can rebase I'll merge this into a branch so we can run CI, and then into master. |
This is useful to know for setting up a hermetic build environment. See bazelbuild#80
Only pass the --namespace parameter to kubectl if the namespace attribute is specified. This will let people specify this in the deployment template instead. See bazelbuild#81
|
Rebased. I just addressed you comment for the README. |
|
Test this please (for Bazel CI) |
|
Hmm, something is broken. I'm seeing: |
| @@ -23,5 +23,4 @@ function guess_runfiles() { | |||
|
|
|||
| RUNFILES="${PYTHON_RUNFILES:-$(guess_runfiles)}" | |||
|
|
|||
| kubectl --cluster="%{cluster}" --namespace="%{namespace}" delete \ | |||
| -f %{unresolved} | |||
| kubectl --cluster="%{cluster}" %{namespace_arg} delete k8s/...-f %{unresolved} | |||
There was a problem hiding this comment.
k8s/... looks like the problem
|
Sorry about it. No idea how this ended up there. Thanks for testing all ops too. Fixed and rebased. |
|
Oh, you merged already. Thanks for cleaning it up. |
|
Thanks for the PR :) |
