Skip to content
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

Add A2 support to the inspec-compliance toolset #2963

Merged
merged 2 commits into from
Apr 19, 2018

Conversation

jquick
Copy link
Contributor

@jquick jquick commented Apr 16, 2018

This is the initial support for Automate 2 with the compliance tools. This supports login/upload/download/exec/logout calls accordingly.

Signed-off-by: Jared Quick [email protected]

@jquick jquick requested a review from a team as a code owner April 16, 2018 14:01
@jquick jquick force-pushed the jq/compliance_a2_support branch from 4de2de9 to ccf178d Compare April 16, 2018 14:27
@@ -18,12 +19,15 @@ class API
# return all compliance profiles available for the user
# the user is either specified in the options hash or by default
# the username of the account is used that is logged in
def self.profiles(config)
def self.profiles(config) # rubocop:disable PerceivedComplexity, Metrics/CyclomaticComplexity, Metrics/AbcSize, Metrics/MethodLength
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're getting dinged for these things because it's past time to treat API as an abstract parent class and make concrete subclasses for the differing targets; API_Compliance, API_Automate, and API_Automate2. Almost all of the conditional code you had to expand here goes away.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I really wanted to break up a lot of this code into sub classes like this. I will take a shot at it and see if I can fit in into the timeline.

Copy link
Contributor Author

@jquick jquick Apr 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Started down this path a bit tonight, its much cleaner but there is a LOT to do here. We call the compliance stuff all over and will need to be re-done with instance classes and caching. I will create a ticket to get this refactor in as its needed but out of scope for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#2964 added, thanks for the feedback.

@jquick jquick force-pushed the jq/compliance_a2_support branch 2 times, most recently from 040a721 to 9fc0032 Compare April 16, 2018 20:47
@jquick jquick force-pushed the jq/compliance_a2_support branch from 9fc0032 to 8acf148 Compare April 16, 2018 21:02
@jquick
Copy link
Contributor Author

jquick commented Apr 19, 2018

Confirmed with @vjeffrey that these changes are working as expected.

Copy link
Contributor

@chris-rock chris-rock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @jquick

@jquick jquick merged commit 33fc155 into master Apr 19, 2018
@jquick jquick deleted the jq/compliance_a2_support branch April 19, 2018 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants