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

Case helper #37

Merged
merged 9 commits into from
Sep 22, 2017
Merged

Case helper #37

merged 9 commits into from
Sep 22, 2017

Conversation

npratley
Copy link

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
Copy link
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

@npratley
Copy link
Author

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
Copy link
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

@npratley
Copy link
Author

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants