Add cf client login to client library

[svenXY]

Implements parts of #172:

client.init_with_cf_config() will read a config file generated with cf login and use endpoint and token from there

The problem was that the constructor for CloudFoundryClient already expects the endpoint and uses the endpoint to retrieve further information. Therefore, the constructor already needs to know that it needs to parse the file to get the endpoint.

[antechrestos]

@svenXY Hi

thank you for this contribution. Maybe I would rather have the approach of a static method in ClodfoundryClient named build_from_cf_config that read the file, the token, instanciate the client according to the info, init it with the read values and return it once built.

What do you thnk of it?

[svenXY]

I followed the way you implemented it. In all of your use cases you first call the class constructor with an endpoint as the first required argument and then a login method.

I agree that passing a string instead of an endpoint is sub optimal but at least it still follows the general pattern...

But if you insist on using the static method I can come up with some code until tomorrow - even if I still think that it changes the patterns.

[antechrestos]

I agrree it changes the pattern; otherwise it introduce an unclear implementation

From what I see, you could follow the same pattern by

• building with target endpoint (developper still should provide it)
• int_with_cf_config(path: Optional[str]): reads the config file, checks that the target endpoint is the good one, sets token

What do you think of it?

[svenXY]

Ok. Sounds like a compromise. I'll see what I can come up with

[svenXY]

Good morning @antechrestos,

thinking about this again, I'm still not sure if this is the right way to do it.

Your proposed solution would mean that

• only the token will be read from the config file
• the endpoint still has to be provided, even though the config file already contains the correct endpoint
• and most probably that you either
  • expect it to fail if provided endpoint and endpoint in config file differ (as the one given will then most probably be wrong)
  • or silently use the correct one from the config file

That sounds so wrong to me... I cannot see where this makes the implementation any clearer. Besides, it forces the developer to provide data (endpoint) again even though it is already there.

The problem imho is really that CloudFoundryClient.__init__() already expects the endpoint as a required argument to be able to call _get_info() on it. You could change the constructor and make target_endpoint an optional argument. Then, when no endpoint is provided, the constructor could already try to parse an endpoint from a config file (with an optional path argument for non-default config locations).

That would make the implementation clear again: either provide an endpoint - but then really use the one provided - or don't provide one - then use the one from config file.

The drawback here would be that the constructor of your main entry point would change, but maybe this can be implemented downward-compatible.

I would either prefer that or - sigh - use the static method you proposed from the beginning, even if it breaks the pattern. In the end it is not me who gets questions and issues...

[svenXY]

OK, I tried making target_endpoint optional and the tests succeed with no changes at all.
Now - me at least - I like the implementation as it is now. What do you think?

[antechrestos]

@svenXY good morning.

The things I was not clear enough; the client has the responsibility to provide an agnostic implementation reflecting the way the api is made ie

• client is the entry point to access the api (v2 and v3)
• each call reflect a call as described in the documentation
• provides oauth2 implementation

There is a provided implementation for shell use provided yet this is not the main aim of the library: this library shall be agnostic of the use and allow developper to use it in a python backend , robotframework tests (my first use case).

This contribution brings the following thing: it breaks the agnostism and link the python library with a cli tool. That is why I though a static method (or a method in the package or elsewhere) could do the trick. Because, behing honest, this is not a feature this is just a helper...

That is why in a functional point of view this need is to build a client based on info provided in a file ie

• read the file
• build the client so as to let him read cf info (logging endpoint, oauth2 endpoint - used by refresh token)
• set the access token

[svenXY]

Where does making target_endpoint optional harm this?

[svenXY]

Hi @antechrestos, I opened #175 which implements the staticmethod you proposed as an alternative

[svenXY]

I gusss this one should be closed