Skip to content

[break] Consistently resolve binary aliases in service endpoints#128

Closed
pkoenig10 wants to merge 1 commit into
palantir:developfrom
pkoenig10:binaryAlias
Closed

[break] Consistently resolve binary aliases in service endpoints#128
pkoenig10 wants to merge 1 commit into
palantir:developfrom
pkoenig10:binaryAlias

Conversation

@pkoenig10

Copy link
Copy Markdown
Member

See #99 (comment)

cc @iamdanfox @ferozco

Before this PR

Binary alias return types are resolved but binary alias parameters are not.

After this PR

Both binary alias return types and parameters are resolved.

@pkoenig10 pkoenig10 requested a review from a team as a code owner December 4, 2018 20:00
@pkoenig10 pkoenig10 changed the title Consistently resolve binary aliases in service endpoints [break] Consistently resolve binary aliases in service endpoints Dec 5, 2018
@pkoenig10

Copy link
Copy Markdown
Member Author

I think it is a bit strange to resolve binary types at all. Aliases in conjure provide type safety, but by resolving binary aliases we're explicitly removing the type safety.

Given that there is only a single type of a streaming request/response, I think it would make more sense to not resolve binary types meaning that only the primitive binary type with generate a streaming input/output.

@ferozco

ferozco commented Dec 6, 2018

Copy link
Copy Markdown
Contributor

You're right that its strange. I think that we should think through the implications of this change and what we want the end outcome to be. Its worth taking a step back and thinking through the wider usage of binary within the language and the peculiarities of how java currently handles it. Happy to spend some time and chat through different approaches.

bulldozer-bot Bot pushed a commit that referenced this pull request Jan 9, 2019
## Before this PR
For jersey service endpoints returning a binary alias type, the return type was resolved correctly (to  `StreamingOutput` or `Response`) but the correct `@Produces` annotation was not applied.

## After this PR
The `@Produces` annotation is applied to endpoints returning binary alias types.

See discussion about resolving binary types in #128.
@dansanduleac

Copy link
Copy Markdown
Contributor

Closing this as it's been stale for a while.
Additionally, we're probably going to go with adding logic around the generated binary aliases so they behave as we want, rather than resolving them to the type that already behaves the way we want.

@pkoenig10 pkoenig10 deleted the binaryAlias branch April 11, 2019 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants