Implement a simple helper to search registered peers of a given user.#476
Implement a simple helper to search registered peers of a given user.#476jonasbardino wants to merge 4 commits intonextfrom
Conversation
mig/server/searchpeers.py
Outdated
| """ % {'name': name}) | ||
|
|
||
|
|
||
| if '__main__' == __name__: |
There was a problem hiding this comment.
This doesn't sit comfortably with me.
First, we've generally tried to make CLI tools have a formal main() method. Second, I believe a CLI tool should be introduced in line with our new conventions which, to my understanding would suggest it be placed into the sbin directory.
There was a problem hiding this comment.
Thanks for the input 👍
The reason for this is that it's a ~99% copy/paste of the existing searchusers.py script in the same folder. I made it based on an explicit discussion with our support team colleagues about their (lack of) access to search registered peers of an employee.
Note that I intentionally left it as a draft because I wanted to record that it is something we want but didn't have time to finish it.
I see two paths ahead:
a) we fast-track the inclusion and merge (or manually install) something they can use as-is
b) we postpone it until we have a style-compliant version with unit tests in line with the FHSesque layout .
I can live with either, but agree that the long-term goal is definitely (b). I think we have the concrete use-case solved so not sure how urgently a generally available solution is.
There was a problem hiding this comment.
Hey - thanks for the response, and a 👍 wrt it being a draft.
Appreciate it's meant to immediately satisfy a requirement - given it's a pressing need and knowing that we're aware we'll need to revisit it I think moving forward likely makes sense. In this case it might even be better what's here isn't "pretending" to be a solution into the future and that clarity at the outset is helpful.
There was a problem hiding this comment.
I've completely reworked this PR to use FHSesque layout and added unit tests as we discussed above.
Only leaving the existing unit test suite style alone until we have merged the mechanics-only part of #481 to avoid conflicts.
4fdd422 to
18f82f7
Compare
c326a72 to
98ae982
Compare
| -F FULLNAME Search for full name | ||
| -f FIELD Show only FIELD value for matching users | ||
| -h Show this help | ||
| -I CERT_DN Search for user ID (distinguished name) |
There was a problem hiding this comment.
Did we ever decide if we should use -I or -i for CERT-DN in these scripts?
NOTE: searchusers is using -I createuser is using -i
There was a problem hiding this comment.
I don't think we did, but at least one external user requested consistency :-)
Not sure if there is anything preventing a change (apart maybe from the time required to update our own routines and docs accordingly). Otherwise no protests from me .
…to help site operators quickly detect issues with peer acceptance and End date. This is a completely reworked version in bin/ with matching unit tests in place.
… calls inside the function with return to let only the __main__ handler deal with exit codes.
…ch and use them in tests.
98ae982 to
722fdba
Compare
Implement a simple helper to search registered peers of a given user to help site operators quickly detect issues with peer acceptance and End date.