set_oauth_host_name function contain code that will never execute.

[sqlldev]

docusign-esign-python-client/docusign_esign/client/api_client.py

Line 810 in 330a500

# Derive OAuth Base Path if not given

Correct me if I'm wrong, but this code will never execute.
I would like to use this functionality :(

It's not a blocker or any critical bug, because I can pass oauth_host_name to ApiClient constructor.
Just letting you know this code looks strange.

[geoffpdevsup]

I work in developer support at DocuSign, and as I recall our customers used to be confused by setting this up in demo, missing the step of setting the oauth host, and ending up with the oath request being sent to production. That is the why we raise an error now instead of setting a default value. I have posted a bug to cleanup the unexecuted code.

[geoffpdevsup]

This bug is filed as [DCM-7258] Unexecuted Code in python API client.

[deckar01]

There is an internal method that is still expecting this empty call to implicitly define the oauth server.

https://github.com/docusign/docusign-esign-python-client/blob/26ec78e/docusign_esign/client/api_client.py#L839-L852

Silently defaulting to production is bad, but it can be fixed without discarding the convenience. The underlying problem with the original code was that the else branch unconditionally used the production server. It could require the base path to match production to make it safe and convenient. If the base path is not defined, it should error. If the base path doesn't have a corresponding auth server, it should error also.

---

A slightly related topic: The way oauth_host_name is required across ApiClient methods is inconsistent.

• These methods require it to be passed as an argument every call:
  • request_jwt_user_token
  • request_jwt_application_token
• This method requires it to be set before calling, doesn't accept an arg for it, and validates it is set:
  • get_user_info
• This method assumes it is set without validation, doesn't accept an arg for it, and will try to make a post request to the host name None if it wasn't set:
  • generate_access_token
• This is the method that will error when trying to use the implicit logic that is now unreachable:
  • get_authorization_uri

The first 2 methods that require the positional arg can't be fixed without a major version bump (or 2 depending up your deprecation policies). I'm not sure what the maintainers' stance will be on set-then-call, optional-kwarg, or required-arg, but I would recommend settling on one and make sure all the methods use that pattern in the next (few?) major release(s?). generate_access_token has a bug and should validate the auth server in a patch.