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

Unexpected CLI prompt to save deployment configuration part 2 #17437

Open
ddeepwell opened this issue Mar 10, 2025 · 4 comments · May be fixed by #17519
Open

Unexpected CLI prompt to save deployment configuration part 2 #17437

ddeepwell opened this issue Mar 10, 2025 · 4 comments · May be fixed by #17519
Labels
bug Something isn't working

Comments

@ddeepwell
Copy link

Bug summary

Unfortunately, #17410 which was meant to fix #17409 did not remove the prompt since should_prompt_for_save still returns True. should_prompt_for_save is an alias for app.console.is_interactive where console is a Rich Console which returns true for the is_interactive property (as seen here) when executing the command prefect deploy.

Version info

Version:             3.2.12
API version:         0.8.4
Python version:      3.13.2
Git commit:          826eb1a7
Built:               Mon, Mar 10, 2025 4:36 PM
OS/Arch:             darwin/arm64
Profile:             local
Server type:         server
Pydantic version:    2.10.6

Additional context

No response

@ddeepwell ddeepwell added the bug Something isn't working label Mar 10, 2025
@zzstoatzz
Copy link
Collaborator

zzstoatzz commented Mar 11, 2025

hi @ddeepwell - what is the unexpected behavior you're seeing? with your repro from #17409 i am seeing the CLI prompt for save in all cases where is_interactive is true (controllable via --no-prompt or the underlying rich env var) which seems correct (ie it no longer depends on whether or not templating occurred).

if you're saying that the prompt should not happen, what should the condition be for suppressing it? (if not only is_interactive)

@ddeepwell
Copy link
Author

I would advocate for no prompt to be made, but this might end up being a bigger question about the philosophy within prefect as a whole about when prompting occurs.

Before #17410, there was no prompt for a prefect.yaml file that would not be changed (as in there was no templating). In my mind, this is the desired effect since a prompt would seem to indicate that something is missing in the file. Furthermore, in this situation (using version 3.2.12 where the prompt always exists) selecting yes to the prompt only modifies the format of the prefect.yaml file which may mess with some projects style conventions.

The removed conditional (identical_deployment_exists_in_prefect_file) could be returned but with a modified setting to not change any existing templating within the file. I understand that this is easier said than done.

Alternatively, the wording of the prompt (Would you like to save configuration for this deployment for faster deployments in the future?) could be changed since receiving this prompt when a perfectly correct prefect.yaml file is used seems unclear to the user.

I would also like to avoid the use of --no-prompt since I am new to prefect and want to be guided in my usage (i.e. I don't want to skip something potentially important). However, the guidance is only useful when it is clear.

@zzstoatzz
Copy link
Collaborator

zzstoatzz commented Mar 12, 2025

hi @ddeepwell - thanks for the thoughts! i chatted with @cicdw and we were thinking we should just remove the prompt that offers to save the config entirely, so we don't have to concoct a condition where we ought to ask and we never risk destructive action as you mention. the prompt is likely more annoying than useful in general

what do you think?

@ddeepwell
Copy link
Author

I think that's a good idea! I'd only add that if there was no file, then prompting to create one might be helpful.

@cicdw cicdw linked a pull request Mar 18, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants