Arvados

The required API functionality is being added in #15531. Once that's done, this may be a matter of documenting a curl/arv command for admins to use.

This is soft-blocked on #15762. In the course of updating the user management documentation I discovered some bugs. I have an in-progress documentation branch but it seems like it would make sense to fix the bugs first so that the documentation doesn't have to include complicated explanations of workarounds.

15577-ownership-transfer is ready for a first look. This is entirely a documentation branch, focused on updating and filling in the gaps on the topic of admin user and group management.

Peter Amstutz wrote:

15577-ownership-transfer is ready for a first look. This is entirely a documentation branch, focused on updating and filling in the gaps on the topic of admin user and group management.

Overall, a great update so far! Sorry for the long list. If you think there is a better way to review big doc updates, let me know.

• doc/Rakefile - Is there an arvbox update missing? How is this change used?

• admin/troubleshooting - I think the title that shows up in the panel on the left should more closely match what someone might be looking for. So, "Request ids and logging" might not be very helpful. What about just "Error Logging"?

• admin/troubleshooting, admin/activation - What do you think about changing the names of the files to better match the titles? Could be a bit confusing the way it is now when passing links around.

• install/cheat_sheet - Very confusing file name, and is actually an admin page not an install page.

• admin/migrating-providers - I think "Matching on email address" and "Link accounts" should be under another heading like "Migrating to a new account provider". Maybe the page should be called "Changing account providers"?

• api/methods/users
  • Might as well remove the arguments table under system.
  • Should the user argument under update be marked as required?
  • For the activate and unsetup methods, maybe add a sentence about what it means to be activated/deactivated, or link to somewhere that explains it.

• admin/activation
  • The first sentence under "User activation" should be replaced with an intro sentence like "This section describes the different user states, permissions ...".
  • I think the diagram should come after the list or be removed. The list is a lot more clear to me. If we keep the diagram, the ability to go from step 3 to 5 via setting "is_active" needs to be expressed.
  • In step 2, mention that these are cluster config values and link to the example config.
  • In step 7, the link should say "ownership reassigned". There are also 2 periods after that link.
  • line 58, "User management can be performed through the web using Workbench"
  • line 62, "user" is repeated.
  • "This is executed as a system user, so it bypasses normal read permission checks." Not sure what this means. Is this referring to the user_agreements list method? Is this an implementation detail, can it be removed?
  • line 102, incomplete sentence.
  • The "Auto-setup federated users from trusted clusters" section could use a config example. The second sentence is confusing and implies that is_active is referring to the cluster. Can this be reworked?

• api/methods/user_agreements - Remove the empty arguments tables for list and signatures?

• admin/reassign-ownership - The title can probably be shortened to "Reassign user data ownership".

• config/config.default.yml - Don't we have to rerun generate if this is changed?

• Also, please run tests.

Side note: We mention using workbench a lot. It's not great that we don't have user guide docs to point to that shows how to do some of this. Ideally once transitioned to WB2, we would have more complete docs for this. In the mean time, I think it is worth adding tickets for various gaps related to 15577 (e.g. WB2 user management docs) and adding TODO comments in the textile.liquid files where links to the user guide docs would be appropriate. This way we will have clear way to tie the docs together once they are done.

Eric Biagiotti wrote:

Peter Amstutz wrote:

15577-ownership-transfer is ready for a first look. This is entirely a documentation branch, focused on updating and filling in the gaps on the topic of admin user and group management.

Overall, a great update so far! Sorry for the long list. If you think there is a better way to review big doc updates, let me know.

• doc/Rakefile - Is there an arvbox update missing? How is this change used?

Just a way to avoid waiting for R SDK docs being regenerated every time "rake" is run.

• admin/troubleshooting - I think the title that shows up in the panel on the left should more closely match what someone might be looking for. So, "Request ids and logging" might not be very helpful. What about just "Error Logging"?

Just "Logging" now

• admin/troubleshooting, admin/activation - What do you think about changing the names of the files to better match the titles? Could be a bit confusing the way it is now when passing links around.
• install/cheat_sheet - Very confusing file name, and is actually an admin page not an install page.

The goal was to avoid breaking existing links to individual documentation pages, but it looks like I can add symlinks (and tweak the doc template) to do the right thing.

• admin/migrating-providers - I think "Matching on email address" and "Link accounts" should be under another heading like "Migrating to a new account provider". Maybe the page should be called "Changing account providers"?

Now "Changing upstream login providers"

• api/methods/users
  • Might as well remove the arguments table under system.
  • Should the user argument under update be marked as required?
  • For the activate and unsetup methods, maybe add a sentence about what it means to be activated/deactivated, or link to somewhere that explains it.

I prefer to keep the arguments table, even if empty, for consistency, since it makes it clear there arn't any parameters.

Technically, the 'users' object on update isn't required, because you can do an update with just "is_active=false" and the API server will figure out that is meant to be part of the 'users' parameter.

• admin/activation
  • The first sentence under "User activation" should be replaced with an intro sentence like "This section describes the different user states, permissions ...".

Done.

• I think the diagram should come after the list or be removed. The list is a lot more clear to me. If we keep the diagram, the ability to go from step 3 to 5 via setting "is_active" needs to be expressed.

The list is supposed to the details explanation of the diagram. Each numbered step corresponds to a box on the diagram. The ability to go from step 3 to 5 is documented on step 6.

Obviously that wasn't clear in the presentation. I changed it to a side-by-side layout to try and make the connection more obvious.

• In step 2, mention that these are cluster config values and link to the example config.
• In step 7, the link should say "ownership reassigned". There are also 2 periods after that link.
• line 58, "User management can be performed through the web using Workbench"
• line 62, "user" is repeated.

Done.

• "This is executed as a system user, so it bypasses normal read permission checks." Not sure what this means. Is this referring to the user_agreements list method? Is this an implementation detail, can it be removed?

Took it out.

• line 102, incomplete sentence.

Fixed.

• The "Auto-setup federated users from trusted clusters" section could use a config example. The second sentence is confusing and implies that is_active is referring to the cluster. Can this be reworked?

Reworded this and added a link to the section further down with a config example.

• api/methods/user_agreements - Remove the empty arguments tables for list and signatures?

• admin/reassign-ownership - The title can probably be shortened to "Reassign user data ownership".

Done.

• config/config.default.yml - Don't we have to rerun generate if this is changed?

Fixed.

• Also, please run tests.

Right, that catches the need to run "go generate" and it appears some workbench integration test might be wonky.

Side note: We mention using workbench a lot. It's not great that we don't have user guide docs to point to that shows how to do some of this. Ideally once transitioned to WB2, we would have more complete docs for this. In the mean time, I think it is worth adding tickets for various gaps related to 15577 (e.g. WB2 user management docs) and adding TODO comments in the textile.liquid files where links to the user guide docs would be appropriate. This way we will have clear way to tie the docs together once they are done.

I added a TODO comment on the user-management file and filed #15834 .

15577-ownership-transfer @ ee341f1fc2579fdf28a8b8f07918f9ae195b0727

https://ci.curoverse.com/job/developer-run-tests/1653/

My only nit is that the user activation numbered list formatting could be cleaned up a bit. I think there are some unnecessary newlines (steps 6 and 7) and step 2 is kind of a list...should we make it one? And hopefully as a side bonus there won't be text from the steps under the diagram :)

But otherwise, this LGTM. Thanks!

Passing tests are here: https://ci.curoverse.com/job/developer-run-tests/1654/