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

LogRecord storage at differents levels #4911

Merged

Conversation

jdevillard
Copy link
Contributor

This PR init the capability to choose if we want to store input properties and payload (output) of activity in the Activity Execution Log Record regarding the #4799

This PR is necessary for the UI development :

@jdevillard jdevillard added the elsa 3 This issue is specific to Elsa 3 label Feb 8, 2024
@jdevillard jdevillard added this to the Elsa 3.1 milestone Feb 8, 2024
@jdevillard jdevillard marked this pull request as draft February 14, 2024 17:45
Copy link
Member

@sfmskywalker sfmskywalker left a comment

Choose a reason for hiding this comment

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

Looks good, just a small request to revisit the naming of "persistence strategy" etc. to be more specific in order to indicate that this is about persisting log records. Otherwise, it might be confusing where we think that this is about persistence in general.

@jdevillard jdevillard marked this pull request as ready for review February 17, 2024 17:25
@jdevillard jdevillard marked this pull request as draft February 17, 2024 17:56
Copy link
Member

@sfmskywalker sfmskywalker left a comment

Choose a reason for hiding this comment

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

Very nice, thanks @jdevillard !

sfmskywalker and others added 3 commits February 20, 2024 14:28
Updated various aspects of the PersistenceTab class and its functionality to improve code quality and readability. Simplified the handling of persistence configurations and simplified the use of properties. Transitioned project reference for Elsa.Api.Client from package reference to direct project reference for better development experience in Elsa.Studio.Core.
/**
Because the entire workflow is considered as an activity, the schema must be the same
ie with
"logPersistenceMode": {
               "default": "default",
}
**/

- fix logic to get the default persistence mode working for the whole activity.
@jdevillard jdevillard marked this pull request as ready for review February 20, 2024 21:23
…ord-configuration

# Conflicts:
#	src/modules/Elsa.MassTransit.RabbitMq/Features/RabbitMqServiceBusFeature.cs
…ord-configuration

# Conflicts:
#	src/modules/Elsa.MassTransit.RabbitMq/Features/RabbitMqServiceBusFeature.cs
@sfmskywalker sfmskywalker removed this from the Elsa 3.1 milestone Mar 20, 2024
@sfmskywalker
Copy link
Member

@jdevillard We will merge this once you give us the green light!

@jdevillard
Copy link
Contributor Author

Ok I found the issue about yesterday demo ,

it was one case when you revert Workflow Configuration to "Default" so the value of the property was LogPersistenceMode.Default and not LogPersistenceMode.Include or LogPersistenceMode.Exclude so the information was loosed when the algo have to record or not the data.

So now I think It's ok for me !

thx a lot !

@sfmskywalker
Copy link
Member

Awesome, great job @jdevillard . Many thanks for this!

@sfmskywalker sfmskywalker added this to the Elsa 3.2 milestone Mar 29, 2024
…ord-configuration

# Conflicts:
#	src/modules/Elsa.Workflows.Management/Features/WorkflowManagementFeature.cs
@sfmskywalker sfmskywalker merged commit 2195e43 into elsa-workflows:main Mar 30, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
elsa 3 This issue is specific to Elsa 3
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants