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

KIALI-1766 - Using Swagger to generate Kiali Client #16

Merged
merged 12 commits into from
Dec 5, 2018

Conversation

gbaufake
Copy link
Member

Using Swagger to auto-generate the Files and including Requests for some Metadata on Requests

@gbaufake gbaufake force-pushed the swagger-metadata branch 2 times, most recently from 3b386f3 to a85500b Compare November 20, 2018 20:13
@gbaufake gbaufake changed the title KIALI-1766 POC on Swagger [WIP] KIALI-1766 POC on Swagger Nov 21, 2018
@gbaufake gbaufake force-pushed the swagger-metadata branch 4 times, most recently from 48b190e to b18ded5 Compare November 21, 2018 21:55
- Including Env for Pytest
- Pytest
- Swagger Class
- test_kiali_alternatives (compare old and new request way)
@gbaufake gbaufake changed the title [WIP] KIALI-1766 POC on Swagger KIALI-1766 POC on Swagger Nov 24, 2018
@gbaufake
Copy link
Member Author

This Pull Request included 33 API Tests (one for each REST API available at swagger)

@gbaufake gbaufake changed the title KIALI-1766 POC on Swagger KIALI-1766 - Using Swagger to generate Kiali Client Nov 26, 2018
@mattmahoneyrh
Copy link
Contributor

Consider adding handling/retries for connection errors that appear to be transient and random, during E2E test runs.

@burmanm
Copy link

burmanm commented Nov 30, 2018

Shouldn't the E2E test take care of that? If there's a connection error, you'll get a Python error which can be easily catched.

I don't know why this copy of client removed the error handling, but this is the part from Hawkular's codebase:

        elif isinstance(e, URLError):
            # Cast to HawkularMetricsConnectionError
            ee = HawkularConnectionError(e)
            ee.msg = "Error, could not send event(s) to the Hawkular Metrics: " + str(e.reason)
            raise ee

Those are the cases when there's a connection timeout etc.

@gbaufake
Copy link
Member Author

Shouldn't the E2E test take care of that? If there's a connection error, you'll get a Python error which can be easily catched.

I don't know why this copy of client removed the error handling, but this is the part from Hawkular's codebase:

        elif isinstance(e, URLError):
            # Cast to HawkularMetricsConnectionError
            ee = HawkularConnectionError(e)
            ee.msg = "Error, could not send event(s) to the Hawkular Metrics: " + str(e.reason)
            raise ee

Those are the cases when there's a connection timeout etc.

I have removed on this PR because we wanted to know more than error message... eg: request, url and other metadata that might be useful to discover if the test failed because of an environmental issue or some problem on API.

Maybe I could implement an error handling that can have another.
Any suggestion on this @burmanm?

@burmanm
Copy link

burmanm commented Nov 30, 2018

Why would those casts remove the access to that information? They were just using inheritance, class HawkularConnectionError(URLError):. The idea was to make the client hide certain implementation details in everyday use.

Point is, I think retry should be in the testing code - not the client code. Otherwise the client's retry might hide something that should be catched by tests. The test should determine which failures are acceptable, not the client. That's my opinion.

@gbaufake
Copy link
Member Author

gbaufake commented Dec 3, 2018

This PR depends on kiali/kiali#693 and kiali/kiali#698

@mattmahoneyrh mattmahoneyrh merged commit 601e0ac into master Dec 5, 2018
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.

None yet

3 participants