Datatamer / tamr-client

Programmatically interact with Tamr
https://tamr-client.readthedocs.io
Apache License 2.0
11 stars 25 forks source link

client base_path not included in "post" calls for custom endpoints #123

Closed mrazo-tamr closed 5 years ago

mrazo-tamr commented 5 years ago

🐛 bug report

When using a the Client.post method for a custom api endpoint, the Client's base_api attribute does not seem to be used in creating the final url. This results in an incorrect address for the endpoint.

🤔 Expected Behavior

A Client with base_url defined as, for example "api" should only produce requests of the form <host>:<port>/api/<endpoint>.

😯 Current Behavior

The request is pointing to a url with no base_path, i.e. `:/"

Example error:

Error 405 HTTP method POST is not supported by this URL</title>
HTTP ERROR 405
Problem accessing /transform/profile/<dataset_name>. Reason:
HTTP method POST is not supported by this URL

💁 Possible Solution

Most likely the prepend is simply missing from the code.

Can be worked around by using the absolute path for all post requests, but this might cause an error once a fix is released.

🔦 Context

Hitting several custom endpoints, both on Unify and multiple husks. 2 of the endpoints (dataset truncate and dataset profile) are now in the versioned API in newer Unify versions, so this issue would not be critical after upgrading.

💻 Code Sample

from tamr_unify_client import Client
from tamr_unify_client.auth import UsernamePasswordAuth
from tamr_unify_client.auth import TokenAuth

auth = UsernamePasswordAuth(<user>, <pass>)

unify_internal_api = Client(auth, '<hostname>', base_path='api')

print("BASE_PATH: " + unify_internal_api.base_path)
result = unify_internal_api.post('transform/profile/<dataset>', json={})
print("REQUEST_URL: " + result.url)
print("REQUEST_RESULT: " + result.text)

🌍 Your Environment

RHEL 7 using Anaconda virtual environment

Software Version(s)
tamr-unify-client 0.4.0
Tamr Unify server v0.43
Python 3.6
Operating System RHEL 7
pcattori commented 5 years ago

@mrazo-tamr , you may need a leading AND trailing slash on your base_path. E.g. base_path = '/api/'.

@nbateshaus should we add a BREAKING CHANGES entry to 0.3.0 where the "accepts absolute paths as relative to origin" change was made?


Additionally, if it never makes sense to set a base_path without a leading/trailing /, we should either:

  1. Have a precondition that checks the base_path for leading/trailing / and fails if it doesn't
  2. Allow base_paths without leading/trailing / from the user, but immediately change it to correctly have leading/trailing / when setting it as a property of the client.
nbateshaus commented 5 years ago

I think #2 would be good, even though it would be another breaking change.