Easier ruby use #487
Merged
Easier ruby use #487
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KnVerey
force-pushed
the
easier_ruby_use
branch
2 times, most recently
from
e455ea8
to
8e25558
Compare
KnVerey
force-pushed
the
easier_ruby_use
branch
from
8e25558
to
a9fb7f1
Compare
|
@Shopify/cloudx This is ready for review. @dturn kindly agreed to take over getting it merged. I already manually tested invoking all the tasks with all possible args from the command line, plus requiring only the stated files in the console and successfully invoking the task from ruby. If changes are required after review, some of those tests may need to be repeated, but otherwise this just needs a test deploy using cumulus-cat. We need to document our Ruby interface, both with inline docs and in the readme. That could be added to this PR, but at this point I'd be inclined to save it for Krane. |
dturn
approved these changes
Aug 27, 2019
|
🎩 in my local
😞 I wasn't able to test |
|
I wasn't able to test kubernetes-run in my local, will try to figure it
out
I used:
be exe/kubernetes-run cloud-portal-staging tierstaging-us-east1-2
flush-cache
…On Wed, Aug 28, 2019 at 12:22 PM Jessie ***@***.***> wrote:
🎩 in my local
- kubernetes-deploy ✅
REVISION="4f7d6697fb5a20d4c960cd1cc6530774069489b6" bundle exec kubernetes-deploy cumulus-cat-production tier4 --template-dir=/Users/jessie.ning/src/github.com/Shopify/cumulus-cat/config/k8s/resources --bindings=@/Users/jessie.ning/src/github.com/Shopify/cumulus-cat/config/k8s/runtimes/production-unrestricted.json
- kubernetes-render ✅
REVISION="4f7d6697fb5a20d4c960cd1cc6530774069489b6" bundle exec kubernetes-render --template-dir=/Users/jessie.ning/src/github.com/Shopify/cumulus-cat/config/k8s/resources --bindings=@/Users/jessie.ning/src/github.com/Shopify/cumulus-cat/config/k8s/runtimes/production-unrestricted.json
- kubernetes-restart ✅
REVISION="4f7d6697fb5a20d4c960cd1cc6530774069489b6" bundle exec kubernetes-restart cumulus-cat-production tier4
😞 I wasn't able to test kubernetes-run in my local, will try to figure
it out
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#487>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AC5A4BU7FBTEK4DLPL6CXTLQG3GALANCNFSM4HPLG3MQ>
.
|
test |
jessie-JNing
approved these changes
Aug 28, 2019
timothysmith0609
approved these changes
Aug 29, 2019
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What are you trying to accomplish with this PR?
Make it easier to use our Ruby interface, i.e.
DeployTask,RunnerTask,RenderTaskandRestartTaskby:requires around so that requiring a specific task (e.g.require 'kubernetes-deploy/deploy_task') actually gets you the dependencies you need to run it (and not dependencies you don't need).require kubernetes-deployrequire ALL the tasks so that it still has a purpose in the world where the correct way to require any specific task--including DeployTask--isrequire kubernetes-deploy/TASK.cc @stefanmb
What could go wrong?
The requires could be incomplete, causing the tasks to blow up in production when executing a line my manual tests didn't cover. I tried making the test files only require the code for their respective tasks, but in addition to being a bit of a mess, it was pointless because all but the RenderTask suite need to require DeployTask (i.e. most of the code) for setup purposes.
Todo
Investigate the OJ conflict with Rails apps and consider requiring that file from commonWhile a valid issue, this is not actually related to this PR.After review but before shipping, test the tasks: