Fix JSON decoding error when empty optional is returned.

[tom-s-powell]

Let me know if you'd prefer some kind of helper to be pulled out.

[ferozco]

Thanks for the PR! I think that we may want to make this change in conjure-python-client instead though.

I'd like to keep the generator as simple as possible, and this would further leak details of the requests implementation into to the generic code gen

[tom-s-powell]

@ferozco I thought about this. This would mean moving the Conjure decoding into the client or having the client at least have some knowledge of the return type (assuming you wouldn't want to return None if response.status_code == 204 else response) since you'd have to handle the binary vs JSON case somewhere. Are you fine with that?

[iamdanfox]

@tom-s-powell I've just enabled CircleCI's build forks setting, so you should get builds from now on!

[ferozco]

I think you're right @tom-s-powell, for now this is probably the best way forward

[iamdanfox]

I'd really like to get the proper test cases implemented before we merge this... I think we'll need to build some dedicated endpoints on the verification-server that will return 204s for map, list and set returns (and also alias of map, list and set).

Tracking here: palantir/conjure-verification#38

[tom-s-powell]

@iamdanfox is there a timeline on this change in conjure-verification?

[qinfchen]

@tom-s-powell, there is a pr for this, palantir/conjure-verification#44

[ahggns]

This doesn't work on the test-case for this in conjure-verification 0.10.0 because it is an alias to an optional type. This type visitor is used in a handful of places to determine whether or not a value is optional. Should the visitor also return true if the type is an alias to an optional? Should there be a separate visitor which handles the alias case?

  {
    "type" : "alias",
    "alias" : {
      "typeName" : {
        "name" : "RawOptionalExample",
        "package" : "com.palantir.conjure.verification"
      },
      "alias" : {
        "type" : "optional",
        "optional" : {
          "itemType" : {
            "type" : "primitive",
            "primitive" : "INTEGER"
          }
        }
      }
    }
  }

[ahggns]

@ferozco @iamdanfox I'm not actually sure what the right thing to do is

[ferozco]

We encountered something similar in conjure-core and made: https://github.com/palantir/conjure/blob/develop/conjure-core/src/main/java/com/palantir/conjure/defs/DealiasingTypeVisitor.java . Perhaps we can factor that out into the generator commons and you can then use it here

[ahggns]

👍 would really like to see that in generator-commons

[ferozco]

4.2.0 is out with the new visitor

[ahggns]

Tests are blocking on palantir/conjure-verification#45

[ahggns]

It wasn't that the verification server doesn't accept null, I think its probably that python requests doesn't send a body when the json argument is None, and the verification server's Json parsing doesn't like that:

$ curl -s http://localhost:8000/confirm/receiveRawOptionalExample/0 -XPOST -H 'content-type: application/json'                                                                        
{"errorCode":"INVALID_ARGUMENT","errorName":"Conjureverification:InvalidRequestBody","errorInstanceId":"ec3a4ec0-4bbd-42da-8058-14357fb1014a","parameters":{}}%

[iamdanfox]

Chatting in person, I've filed #18 to make sure we finish off the tests here, without blocking these guys too much!