Skip to content

Moves description of the on_event callback to a separate subsection #5245

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 2 commits into
base: latest
Choose a base branch
from

Conversation

AArdeev
Copy link
Contributor

@AArdeev AArdeev commented Jul 11, 2025

@AArdeev AArdeev requested a review from Totktonada July 11, 2025 13:29
@AArdeev AArdeev self-assigned this Jul 11, 2025
@github-actions github-actions bot temporarily deployed to branch-gh-5244-fix-description-of-app-roles July 11, 2025 13:31 Destroyed
AArdeev added 2 commits July 11, 2025 16:40
Adds links from role creation steps to all subsections mentioned in the steps (configuration schema, validation function, apply function, etc.);
Fixes #5244
#. Define a function that :ref:`applies a validated configuration <roles_create_custom_role_apply>`.
#. Define a function that :ref:`stops a role <roles_create_custom_role_stop>`.
#. (Optional) Define roles from which this custom role :ref:`depends on <roles_create_custom_role_dependencies>`.
#. (Optional) Define the ``on_event`` :ref:`callback function <roles_create_custom_role_on_event_callback>.
Copy link
Member

Choose a reason for hiding this comment

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

If multiple custom roles have the ``on_event`` callback defined, these callbacks are called one after another in the order
defined by roles dependencies.

The ``on_event`` callback returns 3 arguments, when it is called:
Copy link
Member

Choose a reason for hiding this comment

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

I guess, it accepts three arguments.

@@ -238,7 +215,60 @@ This means that all the dependencies of a role should be defined in the ``roles`
You can find the full example here: `application_role_cfg <https://github.com/tarantool/doc/tree/latest/doc/code_snippets/snippets/config/instances.enabled/application_role_cfg>`_.

.. _roles_create_custom_role_on_event_callback:

On_event callback
Copy link
Member

Choose a reason for hiding this comment

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

BTW, there is the 'Specifics of creating spaces' below and it is written before the on_event callback was implemented. We possibly should recommend to use the on_event callback for this scenario, because it should be simpler. Reach me or @grafin for examples if needed.


- ``config``, which contains the configuration of the role;

- ``key``, which reflects the trigger event and is set to:
Copy link
Member

Choose a reason for hiding this comment

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

key and value are too general, they don't give much information for a reader. How about event_name and event_data?


- ``box.status`` if it was triggered by the ``box.status`` system event.
- ``value``, which shows and logs the information about the instance status as in the trigger ``box.status`` system event.
Copy link
Member

Choose a reason for hiding this comment

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

I don't get how it logs anything.

Comment on lines +250 to +271
return {
validate = function() end,
apply = function()
_G.foo = 0
end,
stop = function()
_G.foo = -1
_G.bar = nil
end,
on_event = function(config, key, value)
assert(_G.foo >= 0)
assert(config == 12345)
if(_G.foo == 0) then
assert(key == 'config.apply')
else
assert(key == 'box.status')
end
_G.is_ro = value.is_ro
_G.foo = _G.foo + 1
_G.bar = config
end,
}
Copy link
Member

Choose a reason for hiding this comment

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

What is the idea behind all this arithmetic?

Here we demonstrate how on_event works, right? So, maybe, an example with creating a space schema or writing something into a space would suit better (we need to react to transition from RO to RW)?

Copy link
Member

@Totktonada Totktonada left a comment

Choose a reason for hiding this comment

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

I'm fine with the new structure, thanks!

I left some observations, mostly are not directly related to this particular change. Feel free to handle them separately from this PR.

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

Successfully merging this pull request may close these issues.

Fix description of Application Roles article
2 participants