Delay the connection check until the first request

[pior]

Why

The OozieClient constructor is performing a connection check. This can make testing harder than it should be.

How

• Add an argument to disable the check in the constructor
• Make the test_connection method part of the API

Resolves #63

[kmtaylor-github]

This can make testing harder than it should be.

Can you unpack this a bit? I would assume than any users of OozieClient would simply mock out the entire object in their testing. e.g. https://github.com/Shopify/starscream/blob/master/tests/shopify/tools/test_oozie_gc.py#L29

[pior]

@kmtaylor-github I don't think that expecting users to mock internals of OozieClient to test their own application is reasonable.

Beyond this, a constructor usually only initialize the object and should never fail for external reasons.

[pior]

I'm not sure why CI is failing on unchanged code...

[kmtaylor-github]

I don't think that expecting users to mock internals of OozieClient to test their own application is reasonable.

And I don't think users should be testing a client library at all. I still don't understand your case.

[pior]

We are not testing PyOozie per se, but we would still need to mock OozieClient._test_connection in our functional tests, even though we don't actually perform any actions to Oozie.

[kmtaylor-github]

... even though we don't actually perform any actions to Oozie.

Why do you even create a real instance of OozieClient if you're not expecting to talk to oozie?

[cfournie]

If you have functional tests that don't want to perform Oozie actions but do want to call methods, you can mock the entire client object like @kmtaylor-github suggested and use mock.Mock(spec=OozieClient) to get a mock that throws an exception when calling an API that doesn't exist.

[pior]

I can mock.patch the client or it's internals.

With this PR, my intention was mostly about slightly improving OozieClient by:

• optionally having a constructor that do not fail for external condition
• kind of documenting this connection check with the test_connection=True argument.

[cfournie]

I'm not opposed to this PR. There's likely another situation where a user might want to delay the connection test until a later point, although I can't think of a user right now who needs that. For your immediate problem though, you probably don't need to patch internals given a spec'd mock.

Is there another internal that you found that you needed to mock?

[pior]

@cfournie My comment was misleading, there is nothing else that would need to be mocked.

[kmtaylor-github]

There's likely another situation where a user might want to delay the connection test until a later point

That seems plausible, but I think a cleaner way to accomplish that would be to lazily test the connection on the first request (rather than allowing calls without the check). Something like:

    def _request(self, method, endpoint, content_type, content=None):
        if not self._valid_server:
            self._test_connection()
            self._valid_server = True
        ...

That only kicks the can a short way down the road however, as any API is likely to boil down to a _request call.

[pior]

CI is failing on the typing checks. Tests are passing successfully.