Skip to content
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

User functions guide #252

Open
wants to merge 21 commits into
base: dev
Choose a base branch
from
Open

User functions guide #252

wants to merge 21 commits into from

Conversation

pseusys
Copy link
Collaborator

@pseusys pseusys commented Oct 13, 2023

Description

Adds userguide describing user-defined functions of the framework.

Checklist

  • I have covered the code with tests
  • I have added comments to my code to help others understand it
  • I have updated the documentation to reflect the changes
  • I have performed a self-review of the changes

@pseusys pseusys added the documentation Improvements or additions to documentation label Oct 13, 2023
@RLKRo RLKRo changed the base branch from master to dev October 13, 2023 13:23
@RLKRo RLKRo mentioned this pull request Oct 16, 2023
4 tasks
@RLKRo
Copy link
Member

RLKRo commented Oct 17, 2023

Add links to examples of usage (if such examples exist in our codebase).

If I understood correctly, information about exception handling is going to be in a separate section. In that case we should move all the specific function information into subsections:

User functions guide
--------------------

Overview
~~~~~~~~

Functions
~~~~~~~~~

``Actor`` handlers
==================

...

Exception Handling
~~~~~~~~~~~~~~~~~~

I think we should also sort the functions in the order of their usage frequency.

@pseusys pseusys marked this pull request as ready for review October 20, 2023 09:38
@pseusys pseusys requested review from ruthenian8, RLKRo and kudep October 20, 2023 09:38
@pseusys
Copy link
Collaborator Author

pseusys commented Oct 20, 2023

WARNING: the guide has one TODO left, regarding some information that has not been documented yet.

@pseusys
Copy link
Collaborator Author

pseusys commented Oct 20, 2023

There is still one TODO left!

@RLKRo
Copy link
Member

RLKRo commented Oct 20, 2023

Maybe split into sections
Overview;
Actor handlers;
Script functions (Pre-transition processors, Pre-response processors, Script conditions and condition handlers, Labels, Responses);
Service functions (Extra handlers, Service handlers, Service conditions, Statistics extractors)?

And also reorder sections (service handlers should be before extra handlers; actor handlers should be last).

I think that Pre-transition and Pre-response could be merged into one section since there is no difference between the two besides the point of execution.

And the guide should include information about the most common use cases for functions with examples (e.g. pre-response processors are commonly used to modify response which is done by updating the response attribute of ctx.current_node).
There should also be links to tutorials that use these functions.

@RLKRo
Copy link
Member

RLKRo commented Oct 20, 2023

There is still one TODO left!

Is this about this line?

They are provided in script dictionary (TODO: parser?) in form of Python functions.

I don't understand why that is needed here. Parser is not merged yet and I don't see the connection between the two.

@pseusys
Copy link
Collaborator Author

pseusys commented Oct 22, 2023

There is still one TODO left!

Is this about this line?

They are provided in script dictionary (TODO: parser?) in form of Python functions.

I don't understand why that is needed here. Parser is not merged yet and I don't see the connection between the two.

Some of the functions can only be defined in python diction. AFAIK parser adds support for parsing the dictionary from YAML file. How will the functions (e.g. labels) be represented in the file?


.. code-block:: python

def handler(ctx: Context, pipeline: Pipeline) -> Context:
Copy link
Member

Choose a reason for hiding this comment

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

Should we assume that the API update will be merged before this PR or after it? If yes, the processing signature will be different: functions will return None

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I consider API compatible with the current one.

docs/source/user_guides/user_functions.rst Outdated Show resolved Hide resolved
docs/source/user_guides/user_functions.rst Outdated Show resolved Hide resolved
docs/source/user_guides/user_functions.rst Outdated Show resolved Hide resolved
docs/source/user_guides/user_functions.rst Outdated Show resolved Hide resolved
docs/source/user_guides/user_functions.rst Outdated Show resolved Hide resolved
docs/source/user_guides/user_functions.rst Outdated Show resolved Hide resolved
docs/source/user_guides/user_functions.rst Outdated Show resolved Hide resolved
@RLKRo RLKRo mentioned this pull request Oct 30, 2023
4 tasks
@pseusys pseusys requested a review from ruthenian8 November 10, 2023 09:32
@pseusys pseusys self-assigned this Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants