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

ST2-385 Actions and class file added #1

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

ST2-385 Actions and class file added #1

wants to merge 16 commits into from

Conversation

Greyjacket
Copy link
Contributor

@Greyjacket Greyjacket commented Jan 21, 2020

Cylance Protect Pull Request:

Summary of Changes:
Cylance Protect Actions and Class file added

New Packs
Delete if not a new pack

  • circleci enabled
  • ssh key setup

Checklist:
Please complete the following for the PR to be reviewed.

  • Write up of changes. The idea here is that whomever lands here can figure out where the PR
    is at and what the purpose of the PR is quickly
  • Documentation completed (README, docstrings, etc.) - Refer to the cookiecutter template
  • Changelog Updated
  • Pack Version Bumped
  • JIRA Issue Tagged
  • Demonstrable Sensors and Actions
  • Peer code review
  • Peer validation of actions and sensors running and review of input and output
  • Linting
  • Tests validated and passing
  • py2 (sensors) and py3 support
  • Review requested for PR to the appropriate user
  • Reference to any other work in JIRA or Github

Fixes for Issue(s)/Ticket(s):
Reference the GitHub Issue(s) or open tickets this PR addresses (if any).

@punkrokk
Copy link
Collaborator

@Greyjacket Before you request a review, what of the checklist in the PR can you complete?

@punkrokk punkrokk self-requested a review January 21, 2020 05:22
Copy link

@btackett3 btackett3 left a comment

Choose a reason for hiding this comment

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

One comment on something. I am not sure whether it needs to be changed. Let me know either way. The rest of the code looks good.

try:
response_dict = response[1].json()
except:
return True, {'result': 'Policy successfully changed'}

Choose a reason for hiding this comment

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

Why would an exception here not mean failure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe I had some issues converting the returned json to a dictionary. We check for a 200 in the earlier call to response[0], so we can be certain that the policy has been changed by the time we arrive here.

Copy link
Collaborator

@punkrokk punkrokk left a comment

Choose a reason for hiding this comment

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

  • Need version bump
  • Remove codacy badge and fix circleci badge
  • basic writeup of the purpose of this PR

CHANGELOG.md Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants