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

deployment apply -y option #1135

Merged

Conversation

CAGS295
Copy link
Contributor

@CAGS295 CAGS295 commented Aug 23, 2023

Description

The user prompt to confirm a clarinet deployments apply makes it hard to use in automated scripts. Adding an option '-y' to pre-accept the deployment would allow users to automate deployments.

@CLAassistant
Copy link

CLAassistant commented Aug 23, 2023

CLA assistant check
All committers have signed the CLA.

@sabbyanandan
Copy link
Contributor

@CAGS295: This would be very helpful toggle, for sure! 👏🏽 Thank you for your contribution. One of the Clarinet team members will review and collaborate with you shortly.

Please also share how Clarinet is running in your infra; any info about CI/CD automation can be helpful context. With that, we can explore additional optimizations in this workflow and unlock advanced use cases.

@CAGS295
Copy link
Contributor Author

CAGS295 commented Aug 28, 2023

This is my toy example:
Launch a node.
Wait for the stacks api to be up.
Use block height as a proxy for the node being ready.
Apply deployments. (no way to skip the prompt afaik)
Run tests.
Kill the node.

#!/usr/bin/env bash
set -eou >/dev/null

API_URL=http://localhost:3999/v2/info
DEV_READY_HEIGHT=137

clarinet integrate --no-dashboard >/dev/null &
CLARINET_PID=$!
echo "clarinet PID $CLARINET_PID"

# Wait until API is up
echo "Waiting on Stacks API"
while ! curl -s $API_URL >/dev/null; do
    sleep 1
done
echo "Waiting on burn block height $DEV_READY_HEIGHT"
#Wait until devnet is ready, epoch 2.4?.
while [ "$(curl -s $API_URL | jq '.burn_block_height')" -lt $DEV_READY_HEIGHT ]; do
    sleep 2
done
# Apply extra deployments
clarinet deployment apply -p integration/deployments/bootstrap.devnet-plan.yml
#Run tests
yarn test

kill -s INT $CLARINET_PID

@sabbyanandan
Copy link
Contributor

This is my toy example: Launch a node. Wait for the stacks api to be up. Use block height as a proxy for the node being ready. Apply deployments. (no way to skip the prompt afaik) Run tests. Kill the node.

@CAGS295: This is very helpful! Many thanks for the clarification.

Copy link
Contributor

@lgalabru lgalabru left a comment

Choose a reason for hiding this comment

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

Thanks @CAGS295! The -y option could be confusing, what's your take on replicating the behavior we have in some other subcommands?

    /// Use on disk deployment plan (prevent updates computing)
    #[clap(
        long = "use-on-disk-deployment-plan",
        short = 'd',
        conflicts_with = "use-computed-deployment-plan"
    )]
    pub use_on_disk_deployment_plan: bool,
    /// Use computed deployment plan (will overwrite on disk version if any update)
    #[clap(
        long = "use-computed-deployment-plan",
        short = 'c',
        conflicts_with = "use-on-disk-deployment-plan"
    )]
    pub use_computed_deployment_plan: bool,

@CAGS295
Copy link
Contributor Author

CAGS295 commented Aug 28, 2023

good. If we can settle on a long option, picking the short one should be straightforward. I do lean towards concise long options. Nobody likes flooding the console with many long options.

@CAGS295 CAGS295 force-pushed the dev/deployment-apply-y-option branch from baee07d to 3b37622 Compare August 28, 2023 20:57
@CAGS295 CAGS295 requested a review from lgalabru August 28, 2023 20:58
@CAGS295 CAGS295 force-pushed the dev/deployment-apply-y-option branch from 3b37622 to 4a2f4a6 Compare September 6, 2023 19:00
hugocaillard
hugocaillard previously approved these changes Sep 7, 2023
Copy link
Collaborator

@hugocaillard hugocaillard left a comment

Choose a reason for hiding this comment

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

LGTM!
Thanks for your contribution @CAGS295!

@lgalabru is #[clap(long = "skip-prompt", short = 's')] okay for you?

@lgalabru
Copy link
Contributor

lgalabru commented Sep 7, 2023

A skip prompt does not seem like a good idea to me.
How about adding the following flags:

    /// Use on disk deployment plan (prevent updates computing)
    #[clap(
        long = "use-on-disk-deployment-plan",
        short = 'd',
        conflicts_with = "use-computed-deployment-plan"
    )]
    pub use_on_disk_deployment_plan: bool,
    /// Use computed deployment plan (will overwrite on disk version if any update)
    #[clap(
        long = "use-computed-deployment-plan",
        short = 'c',
        conflicts_with = "use-on-disk-deployment-plan"
    )]
    pub use_computed_deployment_plan: bool,

So if a developer wants to skip prompt, they chose the right flag for forcing the usage of the deployment on disk, or the computed plan.

@hugocaillard hugocaillard force-pushed the dev/deployment-apply-y-option branch from 0d648e6 to 9593725 Compare September 15, 2023 08:36
@hugocaillard
Copy link
Collaborator

@CAGS295 Can you sign the CLA please? #1135 (comment)

Copy link
Contributor

@lgalabru lgalabru left a comment

Choose a reason for hiding this comment

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

Looking great, this enhancement is going to be super useful! Thank you @CAGS295 for bearing with me :)

@hugocaillard hugocaillard merged commit 2cba233 into hirosystems:develop Sep 19, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants