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
REST2 API support #34
Comments
Hi there, I'm very interested in having REST2 support since this version of the API has fixed a couple of annoyances and makes for a better basis for mechanized interaction with RT. One of the major annoyances is that REST 1.0 sends out values in its response that correspond to the translated strings that would end up getting shown to a user on the web interface. For example dates for tickets are localized which makes it annoying to parse and unreliable if we must expect to parse dates for all possible localized patterns. The REST2 API should be somewhat easy to implement (famous last words) and I'm interested in working on this. However from what I can see from the codebase we'd need to decouple a bunch of functions. Do you think that implementing support for REST2 as a different (new) file entirely would make sense? |
Cool!
Honestly, I have no idea how to do this properly. Do you know if the provided API call set is approximately the same and do they have different names and parameters? |
It's very similar yes. You can see a list of API endpoints here: https://github.com/bestpractical/rt-extension-rest2/blob/master/README#L371 So far the endpoints I've looked at were the same except with 2.0 in the URL and they're sending out json instead of some weird format. So response parsing will need to be completely overhauled. Currently, response parsing for api 1.0 is all intertwined in the code.. so that's why I was wondering if a new file would be less efforts. Also for login, it's possible to use user/login as with 1.0 but you can also pair the API up with the other extension https://metacpan.org/release/RT-Authen-Token to use a sort of "per-application" token instead of the user credentials directly. So we'll have to implement this as well |
I've tried to put down some ideas to get a rough plan of what needs to be done for this issue. If there's something that seems useless or extraneous or missing in there, let me know.
|
(I'll enthusiastically support anything that works and receives no serious objection from the community, looking forward to the PR.) |
+1
OTOH testing against a real RT instance ensures that we are compatible. |
@vinzBad I frankly haven't had the time to start on this yet. but the responses here are giving me more motivation :) really nice to learn that the API v2.0 will be built-in to the next major release 👍 So then, like you say, it's really important to build support for this version of the API sooner rather than later. Maybe the places to start would be twofold:
do you think I'm overthinking it?
I'm not super comfortable on that front since I don't do all that much python development on the day-to-day basis, so I'm all ears for suggestions. |
I have several thoughts about this.
RE test-suite and mock data
|
I like the idea of creating a "new" refactored class which not only supports RT's new REST API but also has a better design. |
We could mimick the inputs to functions currently available for the REST1 client, but since REST2 responses differ, we'd have to put in extra effort to be fully compatible. So I feel like making it fully transparent to users who switch between using REST1 and REST2 will be a challenge, except if we change the API for the REST1 class as well. |
You're right. A new REST2 client shold come first. A port für the REST1 interface maybe second. I just learned on the bestpractical forums that there already is a python REST2 implementation: https://gitlab.com/catalyst-cloud/rt-client Maybe there is some inspiration to be found. |
I would be very interested in the REST2 support. Specially, in the token authentication. |
FWIW I have been playing around a bit with the new API. Unfortunately outputs have changed and the documentation is not as good/complete as it looks at first glance IMO. We'll see how this new API evolves. |
ah thanks for pointing that one out. I've tried it out a bit and while the HTTP client works and lets one request some data (I was able to mostly migrate my simple scripts for work), it is rather incomplete since there's no simplified way to interact with a couple different objects. also its testing is rather inexistant (only one test for a query building function). the REST2 API is itself also rather disappointingly still quite buggy and unfinished :( (for example I can't find any way to create links between tickets or merge tickets) but it is mostly there and we could have a lot of functionality up and running with it. so, first things first: what test suite should we chose? What do ppl participating in this issue say about using pytest? |
fwiw the current test file |
As long as it works and the library is not exotic, I don't mind. Both unittest and pytest are fine for me. |
I would prefer pytest :-) |
I'll switch over to pytest for v2 tests in my next work burst. the v1 tests will stay as they are unless someone ends up converting them later. I'll start by just using pytest as a library for the test files and see if running those inside nose2 causes a problem or not. This way, we can keep CI as it is setup currently. on another subject: I've just tried using catalyst-cloud's rt-client lib with an auth token without supplying user and password and while it was successful at grabbing the auth page (which is unnecessary with a token) subsequent requests failed because the token wasn't saved and reused in request headers. |
Note that REST 2.0 API in RT::Extension::REST2 is incomplete when compared to REST 2.0 API in rt 5.0 itself - https://rt.cpan.org/Public/Bug/Display.html?id=134781 |
Thank you @sim0nx for all your work on this feature! 🎉 |
👍 |
I think I finally found documentation for it:
https://metacpan.org/pod/RT::Extension::REST2
AS it is JSON-based it will be easier to implement and less error-prone I guess.
The text was updated successfully, but these errors were encountered: