Skip to content

Adds box.schema.user.enable and box.schema.user.disable methods #5123

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

Open
wants to merge 4 commits into
base: latest
Choose a base branch
from

Conversation

AArdeev
Copy link
Contributor

@AArdeev AArdeev commented May 21, 2025

  • 2 new files ("user_enable.rst" and user_disable.rst") added to the box.schema submodule.
  • Files describe methods that activate and deactivate users.
  • References added to existing files ("box_schema.rst", "user_grant.rst", "user_revoke.rst")

Fixes #5122

…iption to Submodule box.schema

2 new files ("user_enable.rst" and user_disable.rst") added to the box.schema submodule.
Files describe methods that activate and deactivate users.
References added to existing files ("box_schema.rst", "user_grant.rst", "user_revoke.rst"
Fixes #5122
@AArdeev AArdeev requested review from Totktonada, veod32 and tecl1s May 21, 2025 14:43
@AArdeev AArdeev self-assigned this May 21, 2025
@AArdeev AArdeev added documentation Improvements or additions to documentation doc_gap labels May 21, 2025
@github-actions github-actions bot temporarily deployed to branch-gh-5122-add-box-schema-user-methods May 21, 2025 14:45 Destroyed
@github-actions github-actions bot temporarily deployed to branch-gh-5122-add-box-schema-user-methods May 21, 2025 15:13 Destroyed

.. function:: box.schema.user.enable(username)

Activate deactivated user. If :samp:`'{username}'` does not exist, it will be created. If :samp:`'{username}'` is already active, nothing changes.
Copy link

@tecl1s tecl1s May 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Запуск box.schema.user.grant не создаёт пользователей. Вызов box.schema.user.enable сводится к запуску box.schema.user.grant с предустановленными параметрами. Передаваемый при этом параметр if_not_exists=true относится не к созданию пользователя, а к выводу/сокрытию ошибки повторной выдачи прав (ссылка).

box.schema.user.enable('not-exists')
---
- error: User 'not-exists' is not found
...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправил


.. function:: box.schema.user.disable(username)

Deactivate a user. If :samp:`'{username}'` does not exist, an error is returned.
Copy link

@tecl1s tecl1s May 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Под box.schema.user.disable вызывается box.schema.user.revoke с if_exists = true. В случае if_exists = true ошибка отсутствия отзываемых прав не будет выведена не произойдёт.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправил

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В файле есть текст:

Должен существовать пользователь, должен существовать объект, но если задано {if_exists=true}, то ошибки не будет, если у пользователя нет прав.

Он звучит немного нескладно. Может быть подправить его как-то так:
Отсутствие объекта или пользователя приводит к ошибке. В случае отсутствия у пользователя прав на объект, ошибка не возникнет при {if_exists=true}.

@github-actions github-actions bot temporarily deployed to branch-gh-5122-add-box-schema-user-methods May 22, 2025 08:09 Destroyed
@AArdeev AArdeev requested a review from tecl1s May 22, 2025 09:34

:param string username: the name of a user to be deactivated

:return: ``---``
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Может быть по аналогии с добавленным для box.schema.user.enable добавить "(if failure)"?

Вот пример:

box.schema.user.disable('nope')
---
- error: User 'nope' is not found
...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, добавлю конечно

Comment on lines +11 to +12
Initiates the :samp:`box.schema.user.revoke('{username}','usage,session','universe',nil,` :code:`{if_exists=true})`
(see section :ref:`box.schema.user.revoke <box_schema-user_revoke>`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, explain the effect of this call. What does 'deactivated' mean?

You can use usage and session description from https://github.com/tarantool/tarantool/wiki/Privileges-and-Access-Control-explained#all-possible-combinations-of-object-type-and-permission

Comment on lines +11 to +12
Initiates the :samp:`box.schema.user.revoke('{username}','usage,session','universe',nil,` :code:`{if_exists=true})`
(see section :ref:`box.schema.user.revoke <box_schema-user_revoke>`).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the revoke call is splitted to two inline blocks?

image

Also, in my opinion it is long enough to make it a non-inline block of code.


:param string username: the name of a user to be deactivated

:return: (if success) ``---``
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--- is YAML document start marker and it is not really what the function returns.

The function returns nothing -- zero amount of values. just like the following function:

function nop() end

Tarantool admin console encodes return values in YAML with document start (---) and document end (...) markers:

tarantool> nop()
---
...

Please, describe Lua API in terms of Lua values. Here I would write that the function returns nothing.


:return: (if success) ``---``

(if failure) ``error: User 'username' is not found``
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This console output means that the given error is raised. It is different from returning {['error'] = <...>}. Please, clarify in some way that is consistent with other Lua API descriptions.


.. code-block:: lua

box.schema.user.disable (username)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trailing whitespaces are usually discouraged.

The same for enable.


.. code-block:: lua

box.schema.user.disable (username)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, follow the usual Lua code style, here we write foo(), not foo ().

.. code-block:: lua

box.schema.user.disable (username)
---
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a return value. If we want to show a chunk of console session, let's add prompt (tarantool> ) and the document end marker (...). If we want to show a piece from a Lua file, let's remove this YAML artifact.

(if failure) ``error: User 'username' is not found``


**Example:**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH, I don't see what this example adds to the given description. Is it really needed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of disable comments are applicable here too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc_gap documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add box.schema.user.enable and box.schema.user.disable methods description to **Submodule box.schema**
3 participants