Skip to content

limit memory and output #5320

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

Conversation

Moradf90
Copy link

@Moradf90 Moradf90 commented Aug 5, 2021

we use stackstorm as a base platform to execute plugin actions on our product.
the plugins (packs on st2) will be developed by anyone who will integrate with our product.
then we need to limit the execution to maximum memory size and maximum output result.
add two new configurations underperformance:

  • action_max_memory_mb
  • action_max_output_size_mb
    by default, they set to zero, which means unlimited.

@pull-request-size pull-request-size bot added the size/L PR that changes 100-499 lines. Requires some effort to review. label Aug 5, 2021
@CLAassistant
Copy link

CLAassistant commented Aug 5, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ cognifloyd
❌ Morad Faris


Morad Faris seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Kami
Copy link
Member

Kami commented Aug 17, 2021

Thanks for the contribution.

The change overall seems reasonable (I will add in-line code comments later), but can you please provide some context and background on this change?

@cognifloyd cognifloyd added status:need more info status:needs tests status:needs cla A manual tag to note PRs that are otherwise ready, but the CLA bot says the CLA has not been signed. service: action runner labels Oct 5, 2021
@cognifloyd
Copy link
Member

@Moradf90 I just merged in Master.

Would you please add a changelog entry and add a PR description?

@Moradf90
Copy link
Author

Moradf90 commented May 1, 2022

@cognifloyd changelog entry and description added.
i don't know why the Lint check fails :(

@arm4b arm4b added this to the 3.8.0 milestone May 1, 2022
@Kami
Copy link
Member

Kami commented May 1, 2022

This looks like a useful functionality, but it think it would make more sense / be more useful if this value could also be provided / overridden on action basis - making it an python runner parameter.

This way admin / pack author can specify limits on per action basis using immutable default parameter value.

@Moradf90
Copy link
Author

Moradf90 commented May 2, 2022

@Kami done :)

@cognifloyd
Copy link
Member

@Moradf90 Could you sign the CLA please?

@Moradf90
Copy link
Author

@Moradf90 Could you sign the CLA please?

I did

@cognifloyd
Copy link
Member

@Moradf90 Yes, I see that you have signed the CLA. I suspect your otorio.com email is not associated with your github account. The CLA assistant uses the email on each commit to see who has signed the CLA. Could you do one of:

@cognifloyd cognifloyd removed this from the 3.8.0 milestone Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
service: action runner size/L PR that changes 100-499 lines. Requires some effort to review. status:need more info status:needs cla A manual tag to note PRs that are otherwise ready, but the CLA bot says the CLA has not been signed. status:needs tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants