I bet this will not immediately go through QA. Please don't hesitate to nitpick. :) Just as a disclaimer, I didn't want any flags, just appropriate positional arguments for the two contexts. That's why it might look unconventional.
Created attachment 167397 [details] [review] Bug 37025: Add CLI tool to generate/delete api keys for a given patron on a Koha instance To test: 1) Apply the patch. 2) Suggestion: `cp debian/scripts/koha-api-keys /usr/sbin/`. 3) Familiarize yourself with the tool by just running `koha-api-keys`. 4) Test that you can generate an api key with the generate op. 5) Test that you can delete an api key with the delete op. 6) Run `xsltproc /usr/share/xml/docbook/stylesheet/docbook-xsl-ns/manpages/docbook.xsl debian/docs/koha-api-keys.xml`. 7) Use man to verify that the manpage renders as expected (install man if not present, e.g. koha-testing-docker). 8) Run the syntax test for the docbook files `prove -v xt/verify-debian-docbook.t`. 9) Sign off. Or leave a comment on what you thinks needs to be improved.
This is cool. I was able to create and delete API keys for a user. At step #6 I tried running this command `xsltproc /usr/share/xml/docbook/stylesheet/docbook-xsl-ns/manpages/docbook.xsl debian/docs/koha-api-keys.xml` I got this error: warning: failed to load external entity "debian/docs/koha-api-keys.xml" unable to parse debian/docs/koha-api-keys.xml I see xml files in debian/docs/ but none for koha-api-keys I think it might be nice if the delete command would optionally take the client id as the first argument instead of prompting for it after. Then the usage for delete could be one command with three arguments similar to create koha-api-keys delete [ client_id ] [ borrowernumber ] [ instancename ]
Hi Brendan, thanks for testing this. By mistake I thought I had included that docbook file because I saw `koha-api-keys` in the diff. It is missing, though. You're absolutely correct. The delete op doesn't need a borrowernumber, that's why I read the client id from input. In general many scripts are split up into enable|disable pairs to keep their interfaces simple. Might be also an idea for this one. I mainly want the generation part because that I really need.
Created attachment 167524 [details] [review] Bug 37025: (follow-up) Add missing docbook xml for koha-api-keys
BTW Brendan, reading from input also implies that you can just read the client id from STDIN like this: `echo <client_id> | koha-api-keys delete <instance>` So there's not really a need for another argument.
Here's another form I just learned about, just because I haven't seen it until a couple minutes ago: `koha-api-keys delete kohadev <<< <client_id>`
I tested this again, but I got stuck on verifying the manpage this time. I'm using ktd, installed man with `sudo apt-get install man` Step 6 seemed to work now with the xml file that you added. `xsltproc /usr/share/xml/docbook/stylesheet/docbook-xsl-ns/manpages/docbook.xsl debian/docs/koha-api-keys.xml` Note: Writing koha-api-keys.8 I tried the command `man koha-api-keys` but got No manual entry for koha-api-keys Step 8 tests passed `prove -v xt/verify-debian-docbook.t` All tests successful.
I'm intrigued. What kind of use cases would you have for this?
Mainly for remote testing. I have an additional API that requires client credentials for bearer token generation and if I want to quickly reproduce a bug, I always need to generate a pair via the UI. That's why I wanted a solution which I can use to generate my client credentials, run my tests and invalidate the bearer token as well as the credentials all in one pipe.
Afraid I can't even get past the first hurdle: kohadev-koha@kohadevbox:koha(bug_37025_so)$ sudo koha-api-keys generate default 51 kohadev Unknown borrowernumber: '51' Don't know if I'm being a complete fool, do feel free to correct me if that is so! Jake.
Wow, that's weird!
Ah wait, don't use sudo :D
Ah, I see. In production, it would have to be run with sudo for proper access to sbin, so I don't think we can sign this off yet? Jake.
Ah, you're totally right! Also I think this should be wrapped in koha-shell when giving it a second look.
Created attachment 171014 [details] [review] Bug 37025: (follow-up) Wrap calls to koha modules in koha-shell This change should - allow seamless usage when run as root (implied by /usr/sbin). - give explicit control to the user instead of relying on the environment
Quoting on the shell is so annoying -_-
(In reply to Paul Derscheid from comment #16) > Quoting on the shell is so annoying -_- 1000%
(In reply to Paul Derscheid from comment #16) > Quoting on the shell is so annoying -_- My hot tip is don't quote on the shell. Even though it's just a tiny bit of code, you're better off putting it in its own script file and calling that via koha-shell. I've had this same nightmare heaps of times with local devs, and it just wasn't worth trying to escape every scenario. I seem to recall koha-shell does some truly awful things with quoting...