-
Notifications
You must be signed in to change notification settings - Fork 54
Add task to report on code synchronization status #196
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…tructure right but basics are there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @davidsandilands!
This looks pretty good! While we could take this as-is, it might be worth doing some additional refinement to take this from being a fairly raw Ruby script, to being structured in a way that makes it conducive to testing and breaking down into smaller methods.
I would suggest taking a look at how get_peadm_config.rb is modeled. Namely:
-
Wrap the code in a Ruby class
CodeSyncStatus
-
Use a
CodeSyncStatus#initialize
method to save@params
def initialize(params) @params = params end
-
Include a method to run the task,
CodeSyncStatus#execute!
-
At the bottom of the file, invoke
#execute!
using a stanza like:# Run the task unless an environment flag has been set, signaling not to. The # environment flag is used to disable auto-execution and enable Ruby unit # testing of this task. unless ENV['RSPEC_UNIT_TEST_MODE'] CodeSyncStatus(JSON.parse(STDIN.read)).execute! end
That's how we're currently making Ruby tasks testable in peadm.
Note that we should probably switch all of this over to using ruby_task_helper instead sometime soon. We just haven't done that yet. Originally we didn't want to encumber the dependency since we were supporting old versions of PE that didn't work with it, but I don't think we have that constraint anymore.
See here for a simple example of how a Ruby task that uses ruby_task_helper might look.
…ariables to be more consistent
Updated as suggested https://52.156.134.29/ is available for play with the task I checked locally by manipulating and hard setting variables to mismatch and checking failures if an environment is only partially deployed (you can mock this by not waiting for code deploy to finish) As per discussion with @mcka1n added some extra structure which made some checking easier
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidsandilands I left a couple of comments on the Ruby class (just cosmetics)
Overall the logic looks good to me :)
I made the change as part of my commit
This task runs and produces output based on the debug status on the primary server example output below