Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Proposed changes to ensure JSON REST Consistency (Fixes #703) #707

Merged
merged 4 commits into from
Aug 1, 2019

Conversation

axsaucedo
Copy link
Contributor

@axsaucedo axsaucedo commented Jul 18, 2019

WIP: Proposed changes to ensure JSON REST Consistency (Fixes #703)

This pull request contains the proposed changes to modify the microservice API such that for REST requests, it is ensured that the JSON requests stay intact as per the request in #703, as currently the google proto changes int representations into floats (i.e. 9 to 9.0), which modifies the original JSON in ways that then seem to affect potential proxies such like the TFServing proxy.

Once this is agreed, it would be applied across the rest of the APIs. It would also be necessary to assess that it wouldn't affect other wrappers, etc.

@seldondev seldondev added size/M and removed size/L labels Jul 18, 2019
@axsaucedo axsaucedo changed the title Proposed changes to ensure JSON REST Consistency (Fixes #703) WIP: Proposed changes to ensure JSON REST Consistency (Fixes #703) Jul 19, 2019
@seldondev seldondev added size/L and removed size/M labels Jul 24, 2019
@axsaucedo
Copy link
Contributor Author

@cliveseldon I've added some new tests on the funcitons added using the other tests as reference.

@axsaucedo
Copy link
Contributor Author

Confirmed all relevant tests run - as soon as @cliveseldon can confirm tests run locally should be possible to land (as local tests on GRPC fail due to configuration)

@jklaise
Copy link
Contributor

jklaise commented Aug 1, 2019

I can confirm that tests run locally.

@axsaucedo
Copy link
Contributor Author

Perfect, thanks @jklaise

@axsaucedo axsaucedo merged commit db458fa into SeldonIO:master Aug 1, 2019
@axsaucedo
Copy link
Contributor Author

Fixes #458 as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

All numbers get converted to float when sending JSON resulting in unexpected behaviour in Proxies
3 participants