Skip to content

Conversation

@Satbek
Copy link
Contributor

@Satbek Satbek commented Feb 3, 2021

It supports all v1 methods, options, fields. It works via layer over v2 API.
But user can use only one API. http_server will throw an error if user tries to use methods from both APIs.

closes #131

It supports all v1 methods, options, fields. It works via layer over v2 API.
But user can use only one API. http_server will throw an error if user tries to use methods from both APIs.
@Satbek Satbek requested a review from Totktonada February 3, 2021 10:25
Comment on lines +115 to +119
elseif self.__api_version == API_VERSIONS.V2 then
error(
('":%s" method does not supported. Use http-v2 api https://github.com/tarantool/http/tree/master.'):
format(method_name)
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the check http server instance wide? So, different http server instances may be used according to different API versions, right? Is it fit good to the case, when cartridge uses one API version, but a user's application uses another one?

What is the reason of blocking such usage? It would not work appropriately?

Copy link
Contributor Author

@Satbek Satbek Feb 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a check only for one http_server instance, not for all.
Yes, different instances could be used according to different API versions.
Yes, I think it fits good to the case with user and cartridge application servers. We may ask @rosik about it.

Both APIs have documentation and covered by tests. But their simultaneous usage not documented and not tests. So I think there are cases when simultaneous usage can lead to bugs or strange not documented behaviour. Error on such cases prevents it. Also I don't think that simultaneous usage is a good practice. As for me it would not give any advantages and confuses a code with a lot of different method calls, which do the same thing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the clarification! It seems meaningful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rosik said that users usually add application specific routes into the cartridge's http server.

@Totktonada Totktonada mentioned this pull request Mar 24, 2021
@yngvar-antonsson
Copy link

@Satbek should it be closed since http v2 is deprecated?

@Satbek
Copy link
Contributor Author

Satbek commented Nov 1, 2021

no need on it after #134

@Satbek Satbek closed this Nov 1, 2021
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.

Backward compatibility with http-v1 api

4 participants