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

[MILAB-685] Smart cache new #8

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

[MILAB-685] Smart cache new #8

wants to merge 13 commits into from

Conversation

Snyssfx
Copy link
Member

@Snyssfx Snyssfx commented Sep 25, 2024

No description provided.

Copy link

changeset-bot bot commented Sep 30, 2024

🦋 Changeset detected

Latest commit: 7ab845c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@platforma-sdk/workflow-tengo-tests Minor
@platforma-sdk/workflow-tengo Minor
@milaboratories/pl-middle-layer Patch
@platforma-sdk/test Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Snyssfx Snyssfx requested review from dbolotin, PoslavskySV and DenKoren and removed request for dbolotin and PoslavskySV September 30, 2024 13:49
Copy link
Member

@PoslavskySV PoslavskySV left a comment

Choose a reason for hiding this comment

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

Apart from minor comments, all looks good for me. But we need to test it in real blocks.

sdk/workflow-tengo/src/smart.lib.tengo Outdated Show resolved Hide resolved
*
* @param fn: func() - the hook that should be executed before the end of the body.
*/
registerOnBodyEnd: func(fn) {
Copy link
Member

Choose a reason for hiding this comment

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

will it be executed if thee is an exception inside the body?

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope, I guess if any error happens the whole rendered block will become unusable, so I don't think it's too important.

Also, it seems like a good addition, but how to add this? We probably can add it in the workflow's controller

Copy link
Member

Choose a reason for hiding this comment

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

Is it guaranteed that there will be no memory leak? The data stored in KV (for use in cache) will be cleared ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeap, pl-core will delete KV along with the resource

@dbolotin dbolotin changed the title Smart cache new [MILAB-685] Smart cache new Dec 4, 2024
Copy link

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.

2 participants