-
Notifications
You must be signed in to change notification settings - Fork 11
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
chunkedgraph
module documentation improvements and standardization
#129
chunkedgraph
module documentation improvements and standardization
#129
Conversation
caveclient/chunkedgraph.py
Outdated
Timestamp of the operation. | ||
"user_id": int | ||
User who performed the operation. | ||
# TODO not actually sure what the rest of the outputs are, not an admin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this command didn't work for me (even with a non-zero user_id
, so an admin would need to fill this out.
perhaps also worth clarifying who can use this function at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bethany also had the same issue as me - I could open a new issue if this is meant to be available to everyone as the docs currently indicate, or if not, we could clarify here.
Although, if user info is not meant to be accessible to everyone, this info is available in the tabular changelog, so there's already a clunky workaround
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
current user of the clunky workaraound
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We were conservative in what we made available to users to easily follow other peoples work. I think a discussion can be had whether this should be behind admin privileges. For giving the necessary privileges: It is easy to add dataset-specific admins, so it would be straightforward to add the two of you as that (and we should do that). While the data is available in tabular changelogs, one would still have to iterate through all neurons to trace another users actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's fair. for the purposes of this PR, I just added a note to the documentation indicating that this is only supported for admins.
separately, yes, I'd be happy to 1) get access but more importantly 2) talk about opening up this function. I don't care too much, but since there's already a roundabout way to get that info it seems like it would make sense to support for any user (or closing that loophole if not).
I think I've addressed everything - but lmk if any further changes are requested |
Summary
TODO