palantir / conjure-python-client Goto Github PK

What happened?

When a conjure Python client is deserializing a response from a conjure-compliant server and receives an unknown union variant, it fails with:

  File "/conjure_python_client/_serde/decoder.py", line 96, in decode_conjure_union_type
    type_of_union, conjure_type
ValueError: unknown union type myNewUnionVariant for <class 'MyUnionType'>

With the current behavior, it's not possible for clients to opt into ignoring unknown variants for a particular object, and the entire decoding will fail.

If a conjure-compliant server has an endpoint that returns values containing a union type, this means that conjure Python clients will break if the server ever upgrades to a version that starts returning a new union variant. Conjure Java clients don't have this problem, as their deserialized unions have visitors that require explicitly handling unknown types.

What did you want to happen?

The most robust solution would be to change conjure-python generation to include an unknown variants for unions. It currently does so for enums but not for unions. Compare the generated union code for conjure-python and conjure-java:

• https://github.com/palantir/conjure-python/blob/3.18.0/conjure-python-core/src/test/resources/types/expected/package_name/_impl.py#L1506-L1633
• https://github.com/palantir/conjure-java/blob/6.68.0/conjure-java-core/src/integrationInput/java/com/palantir/product/UnionTypeExample.java

As a workaround (since that would be a large change), another option would be to expose a flag to return None for union variants that fail to deserialize rather than failing the entire request.

What happened?

Describe what you've observed and why it's bad (please include whatever stacktraces/version numbers you can).
Clear steps to reproduce the behaviour are always helpful too!

A very common use case for this type of capability is the ability to easily serialize/deserialize typed objects from a shared interface, but we need the ability to be a bit more flexible when deserializing objects into Conjure classes or we're going to need to use a different tool to share interfaces (which would be a bummer since I think its the right tool here).

Example:

Input object (python dictionary):
{ "foo-bar": "baz" }

Desired Conjure Object:

Banana(foo_bar: baz)

Currently - this will result in an empty Banana(FooBar: None) object which can potentially be misleading.

What did you want to happen?

I realize that having strict standards is generally a good thing, especially in an area with mostly solved problems (like IR object representation), so I am not proposing any changes in the default behavior.

Instead, I am arguing there should be a separate, non-standard set of "DecodingOptions" that should be available to the user upon request - with, for example, an option to loosen restrictions around deserialization and allow fields with hyphens to be able to deserialize into objects.

An implementation of what this might look like: PR- #131

Hey,

It seems that this expects JSON error messages to be returned at all times, which doesn't fair well when that's not the case (e.g. nginx HTML error page).

Example stacktrace:

ValueError: No JSON object could be decoded 

Traceback (most recent call last): 
[...redacted...]
  File "/tmp/conda-80426e79-fa6e-45d6-988d-c396ee1b1729/real/envs/conda-env/lib/python2.7/site-packages/conjure_python_client/_http/requests_client.py", line 80, in _request 
    detail = e.response.json() 
  File "/tmp/conda-80426e79-fa6e-45d6-988d-c396ee1b1729/real/envs/conda-env/lib/python2.7/site-packages/requests/models.py", line 896, in json 
    return complexjson.loads(self.text, **kwargs) 
  File "/tmp/conda-80426e79-fa6e-45d6-988d-c396ee1b1729/real/envs/conda-env/lib/python2.7/json/__init__.py", line 339, in loads 
    return _default_decoder.decode(s) 
  File "/tmp/conda-80426e79-fa6e-45d6-988d-c396ee1b1729/real/envs/conda-env/lib/python2.7/json/decoder.py", line 364, in decode 
    obj, end = self.raw_decode(s, idx=_w(s, 0).end()) 
  File "/tmp/conda-80426e79-fa6e-45d6-988d-c396ee1b1729/real/envs/conda-env/lib/python2.7/json/decoder.py", line 382, in raw_decode 
    raise ValueError("No JSON object could be decoded") 

ValueError: No JSON object could be decoded 

This prevents us from finding what the root cause of the error is.

I've been copying around this kind of function to make service initialization easier, either the process to initialize a service should be easier or a function like this should be exposed by conjure-python-client.

def create_service(uris, service):
    # type: (List[str], Type[T]) -> T
    config = httpremoting.ServiceConfiguration()
    config.uris = uris
    agent = '{}_conjure_client'.format(service.__name__)
    return httpremoting.RequestsClient.create(service, agent, config)

https://wiki.python.org/moin/NewClassVsClassicClass

It generates this:

    @property
    def property(self):
        # type: () -> str
        """Bar foo"""
        return self._property

    @property
    def second_thing(self):
        # type: () -> str
        """Foo bar"""
        return self._second_thing

Which breaks on line 7, when it tries to use the earlier method called property as a decorator.

It should probably import __builtins__ at the start and use @__builtins__.property instead. The same would be true for other builtins.

Hey,

https://github.com/palantir/conjure-python-client/blob/develop/conjure_python_client/_serde/decoder.py#L50

if the field is missing from the object in the line above instead of throwing exceptions, we call check_null_field which in turn just creates an {} for the field instead of throwing an error.

I could not figure out how optional fields are meant to work here so did not PR but to me it sounds like if it anything but an optional we should throw an exception.

Will PR a fix that i think will work but not sure.

What happened?

I was trying to integrate the conjure generated python classes into my Python3.7.5 environment, and hit an issue. It seem that enum34 breaks newer versions of setuptools as discussed in detail here and here. As both conjure-python-client and the generated classes themselves list enum34 as a dependency Installing the package into my project breaks pip.

For now I had to end up manually scripting/editing the dependency in my generated class files, and "vendor" "conjure-python-client" which is clearly not ideal.

What did you want to happen?

Perhaps it would be an easy fix to replace the dependency with a conditional such as '"enum34; python_version < "3.4"'

Many thanks

What happened?

Attempting to send over a datetime field does not marshall correctly. The error returned is:

TypeError: Object of type 'datetime' is not JSON serializable

What did you want to happen?

datetime is a very common Python type. We should marshall this for the user instead of requiring a pre-conjure map / unmap.

What happened?

Sending requests to an overloaded server retries up to the configured limit and then gives up.

What did you want to happen?

Limit the number of concurrent outbound requests to the server based on previous responses from the server. When the limit has been reached, wait until outstanding requests finish before sending new requests.

The conjure-java client switched to adaptive limits to cooperate with the server to send requests at a reasonable rate. Can we implement an AIMD limit for this client?

recommended by the maintainer of conda-forge

I'd assume that URL parameters should be encoded automatically in the same way they are for Java clients.

What happened?

For the following conjure spec:

types:
  definitions:
    default-package: com.company.product
    objects:

      Recipe:
        fields:
          name: RecipeName
          steps: StringList

      StringList:
        fields:
          items:
            type: list<string>
            safety: safe

      RecipeName:
        alias: string
        safety: safe

services:
  RecipeBookService:
    name: Recipe Book
    package: com.company.product
    base-path: /recipes
    endpoints:
      createRecipe:
        http: POST /
        args:
          createRecipeRequest:
            param-type: body
            type: Recipe

With the following python code

from conjure_python_client import ConjureDecoder, ConjureEncoder

decoder = ConjureDecoder()
incorrect = decoder.decode({"name": "test", "steps": ["test"]}, Recipe)
print(incorrect)

I get the following output:

Recipe(name='test', steps=StringList(items=[]))

So there was a silent failure that inserted a blank object and a blank array

What did you want to happen?

An error should be thrown, same as if you tried to decode a string type as an int.

I would expect an error to be thrown since this is an incorrect type provided

What happened?

Service mesh URIs are used between conjure services in k8s with a special 'mesh-' prefix. Without special handling, requests for these URIs will fail with an unsupported URI scheme.

What did you want to happen?

Follow the behavior implemented by conjure clients for other languages (removing the prefix and disabling retries):

• dialogue: palantir/dialogue#1048
• conjure-rust-runtime: palantir/conjure-rust-runtime#81

What happened?

I am using a server conjure service and on exceptions I do not see the errorType deserialized properly.

When a remote exception is throw I get back a payload like this:

  "cause": None,
  "stackTrace": [ REMOVED FROM ISSUE ],
  "errorType": {},
  "args": [],
  "errorInstanceId": "713f5344-e299-41d6-89ff-0244a9a6a8fb",
  "logMessage": "ServiceException: INVALID_ARGUMENT (Workflow:AlertMissingInfo)",
  "message": "ServiceException: INVALID_ARGUMENT (Workflow:AlertMissingInfo)",
  "parameters": [],
  "suppressed": [],
  "localizedMessage": "ServiceException: INVALID_ARGUMENT (Workflow:AlertMissingInfo)"

The errorType parameter is missing above.

My code is not able to deduce the server side errorType for conditional error handling:

if alert_response.failure_response:
        service_exception = alert_response.failure_response.service_exception
        if service_exception['errorType'] != 'Workflow:AlertMissingInfo':
            raise ValueError(f'workflow alert missing info')

Server side is using conjure.java 4.5.0. Client side is using conjure-python-client 1.7.3 build py37_0.

What did you want to happen?

I'd expect errorType to deserialize properly from a Java Conjure service.

Release with #142 resulted in below issue. We should check that conjure_type.code exists

File "/Users/stheer/.local/pipx/.cache/bc093e9748e7285/lib/python3.8/site-packages/conjure_python_client/_serde/decoder.py", line 128, in decode_conjure_union_type
    if 'type_of_union' in conjure_type.__code__.co_varnames:
AttributeError: type object '<...>' has no attribute '__code__'

Conjure types cannot currently be used in collections such as sets given the only have implementations of __eq__, not __hash__.