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

Case helper #37

Merged
merged 9 commits into from Sep 22, 2017

Conversation

Projects
None yet
3 participants
@npratley
Contributor

npratley commented Sep 14, 2017

I’ve added a helper class for interacting with cases as a higher level. It wraps the existing get_case and create_case methods and in both cases returns a Case instance rather than a requests Response instance. These methods have not been touched to backwards compatibility should not be affected.

There is a sample file included (test-case-create__case-helper.py) which can be compared with test-case-create.py to show how the usage changes.

The next step would be to wrap some of the other methods, e.g. to make it easier to work with case tasks, but I’ll wait for some feedback on this approach before going further.

It looks like there are some duplicated commits - I originally forked from master, and then realised I should have forked from develop, and when I tried to rebase against develop I messed something up. Sorry about that.

@nadouani

This comment has been minimized.

Show comment
Hide comment
@nadouani

nadouani Sep 14, 2017

Contributor

Hi @npratley

This is a great job, I like the idea and the implementation.

We all know that returning the Responses from the API Client is not the best way, and we were thinking of a new API class without breaking the compatibility.

What you suggest is really good, and solves what we want:

  • Add custom exceptions for better error handling
  • Get rid of the Response object returned by methods

I'm personally ok with what you started. Let's wait for other's opinion ;)

Thanks

Contributor

nadouani commented Sep 14, 2017

Hi @npratley

This is a great job, I like the idea and the implementation.

We all know that returning the Responses from the API Client is not the best way, and we were thinking of a new API class without breaking the compatibility.

What you suggest is really good, and solves what we want:

  • Add custom exceptions for better error handling
  • Get rid of the Response object returned by methods

I'm personally ok with what you started. Let's wait for other's opinion ;)

Thanks

@npratley

This comment has been minimized.

Show comment
Hide comment
@npratley

npratley Sep 18, 2017

Contributor

I've rebased onto develop since release-1.3.0 was merged, and fixed conflicts. Removed the duplicated commits while I was at it.

Contributor

npratley commented Sep 18, 2017

I've rebased onto develop since release-1.3.0 was merged, and fixed conflicts. Removed the duplicated commits while I was at it.

@nadouani

This comment has been minimized.

Show comment
Hide comment
@nadouani

nadouani Sep 18, 2017

Contributor

Hi @npratley
As I said earlier, your solution looks great, so go ahead if you still have things to commit. What features are you planning to implement on that helper

Contributor

nadouani commented Sep 18, 2017

Hi @npratley
As I said earlier, your solution looks great, so go ahead if you still have things to commit. What features are you planning to implement on that helper

@npratley

This comment has been minimized.

Show comment
Hide comment
@npratley

npratley Sep 18, 2017

Contributor

Hi @nadouani , the goal is to create a higher-level interface but to be honest I’m not really sure what that should look like, so I wanted to submit a small change rather than biting off too much. That’s probably it for the case helper class for now while I figure out what to do next.

I commented on #12 that I can take that over to track the overall work (or create a new issue if it’s not quite the same thing?).

Contributor

npratley commented Sep 18, 2017

Hi @nadouani , the goal is to create a higher-level interface but to be honest I’m not really sure what that should look like, so I wanted to submit a small change rather than biting off too much. That’s probably it for the case helper class for now while I figure out what to do next.

I commented on #12 that I can take that over to track the overall work (or create a new issue if it’s not quite the same thing?).

@nadouani nadouani added this to the 1.4.0 milestone Sep 22, 2017

@nadouani nadouani merged commit aff090e into TheHive-Project:develop Sep 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment