st2 icon indicating copy to clipboard operation
st2 copied to clipboard

Expose timeout parameter to packs.install on st2 client pack install

Open hnanchahal opened this issue 4 years ago • 8 comments

Added timeout parameter for pack install st2 client run to help with long running installs. This was already expose in packs.install action but st2client was not updated to reflect the change.

hnanchahal avatar Mar 02 '21 22:03 hnanchahal

I believe you also need to update st2common/st2common/openapi.yaml.j2, specifically PacksInstall object and also the corresponding API handler code in st2api/st2api/controllers/v1/packs.py.

Kami avatar Mar 02 '21 23:03 Kami

Thanks for the contribution.

Overall, I'm fine with this change, but as mentioned above, it needs some more work + ideally at least the unit test for the API layer (that one shouldn't be too hard to add).

Kami avatar Mar 02 '21 23:03 Kami

We just reformatted the code with black. (Hooray!) And this PR got caught in the cross fire too. (Arrgh!) Please merge master into your branch, resolve the conflicts (Ouch! Sorry!), and reformat with black (I recommend running pre-commit install after you've merged master so that black reformatting happens automatically on commit).

cognifloyd avatar Mar 07 '21 07:03 cognifloyd

Thanks for contributing to this issue. As it has been 90 days since the last activity, we are automatically marking is as stale. If this issue is not relevant or applicable anymore (problem has been fixed in a new version or similar), please close the issue or let us know so we can close it. On the contrary, if the issue is still relevant, there is nothing you need to do, but if you have any additional details or context which would help us when working on this issue, please include it as a comment to this issue.

stale[bot] avatar Jun 09 '21 02:06 stale[bot]

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.

:white_check_mark: cognifloyd
:x: hnanchahal
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Sep 06 '21 14:09 CLAassistant

@hnanchahal I just merged in master and dealt with reformatting. Would you please sign the CLA so we can move this PR forward?

cognifloyd avatar Apr 01 '22 19:04 cognifloyd

Rephrasing what Kami said, to finish up this PR, you need to add the server-side / API changes. Though the packs.install action has a timeout parameter, the API layer can't pass it on until it knows about it.

So, you've added the CLI changes and the spec changes, the last pieces are:

  • change st2api/st2api/controllers/v1/packs.py (PackInstallController)
  • add API tests in st2api/tests/unit/controllers/v1/test_packs.py

And sign the CLA :) If you can get this done within a week or two I'll merge it in time to release 3.7.0. Cheers!

cognifloyd avatar Apr 02 '22 00:04 cognifloyd