Skip to content

Comments

Added not-found error message for hostname search#47

Merged
cmeissner merged 5 commits intocodeaffen:developfrom
mattiasa:hostname_search_exception
Sep 2, 2021
Merged

Added not-found error message for hostname search#47
cmeissner merged 5 commits intocodeaffen:developfrom
mattiasa:hostname_search_exception

Conversation

@mattiasa
Copy link
Contributor

The following call raises a PHPyPAMException.

pi.get_entity(controller='addresses', controller_path=f"search_hostname/{hostname}/")

I would expect the library to raise PHPyPAMEntityNotFoundException instead. This PR fixes this by adding the error returned from phpipam to the expected list.

@mattiasa mattiasa changed the title Added not-found error message for address search Added not-found error message for hostname search Aug 31, 2021
@cmeissner
Copy link
Member

@mattiasa many thanks for submitting this pull request. May I ask you to provide a first test case for that exception. We would be very appreciated if you can do this.

@cmeissner cmeissner linked an issue Aug 31, 2021 that may be closed by this pull request
@mattiasa
Copy link
Contributor Author

Sure, I can do that. I looked around among the tests and couldn't find anything that tested the actual server responses but I'll come up with something.

@cmeissner
Copy link
Member

Yeah unfortunately we don't have tests for all the exceptions yet. But this pr is a good starting point for that.

Copy link
Member

@cmeissner cmeissner left a comment

Choose a reason for hiding this comment

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

The change isn't complex and it works for me.

Refactor exception.py to make `_NOT_FOUND_MESSAGES` available outside
exception definition.
Add test method for testing not found exceptions. Iterate over a list of
dicts with test data.
Add fixture for the new test case for not found exception.
@cmeissner
Copy link
Member

@mattiasa I had some time to implement a first version for testing the PHPyPAMEntityNotFoundException. Hope this is okay for you.

@cmeissner cmeissner self-requested a review September 2, 2021 14:52
@mattiasa
Copy link
Contributor Author

mattiasa commented Sep 2, 2021

Yea, thanks a lot for writing the tests. I didn't even get to a point where I could run the test suite before I had to put it aside for some other things.

@cmeissner cmeissner merged commit a70ec80 into codeaffen:develop Sep 2, 2021
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.

Search for non-existing hostname results not in correct exception

2 participants