blag commented on PR #23335:
URL: https://github.com/apache/airflow/pull/23335#issuecomment-1112730294
> 1. the message is going to be repeated multiple times. when people will
run it. So what I am counting on is the time of displaying the message to be
multiplied by the number of times people see it.
This just seems like worse UX, but okay. 🤷
> If we follow your min 30 seconds and take it literaly, this means that any
warning printed is never read and understood (because people had no time to
read it).
I apologize if I haven't been clear enough. I proposed a few different
scenarios.
The easiest scenario is to simply emit a colorful warning and let the user
discover it if/when things fail. The steps for that is:
1. Print a colorful warning ("Breeze is out-of-date. If this fails, try
updating breeze with (`breeze self-update`) and run this again")
2. Carry on.
Another scenario is when we're just trying to delay/annoy the user into
updating breeze, or pulling in changes from `main`, or whatever - cases where
we don't actually need or care about user input, but we do want to apply some
pressure. The steps in that case are:
1. Print a colorful warning (eg: "If this fails, try updating breeze or
Airflow and try again. You can disable this wait by setting
`DONT_WAIT_JUST_GO=...` or specifying the `--dont-wait-just-go` option")
3. Wait for a delay (this does not have to be 30 seconds, it can be lower)
4. Continue with what the command doing in the first place (eg: Run breeze
without updating)
The user has to presumably stare at the warning for the duration of the
delay, presumably reading and pondering over the correct course of action to
take. And if they do not, and the command continues and eventually fails, the
user has hopefully taken note of the warning and tries the command again with
changes suggested from the warning (eg: pulling in changes from `main`, setting
an environment variable and rerunning the command, or running the command with
an additional flag). And even in the case that the user completely ignore the
warning, switches context to something else during the delay, and eventually
switches back and sees that the command has failed, they can scroll right back
up to where the warning is printed in nice colors (bring the users attention to
it), and that will hopefully give them enough of a cue to implement the
suggested changes and try the command again. Since we don't actually care about
the user's input here, we don't bother asking them for it.
The next scenario is when we do actually care enough to ask for user input.
In this case, I think we should avoid timeouts if at all possible. In that
case, the step would be:
1. Ask for user input and hang until they answer (eg: "Recursively delete
'/'? y/n")
Finally, as a compromise, or in a such a case that we must timeout while
waiting for input (although I really can't think of a good use case for that
myself, I'm willing to consider that the possibility may exist), we can add a
timeout of 30 seconds to give the user time to read the text, ponder their
decision, and either cancel the command (to presumably immediately rerun it
with changes) or cajole the command into continuing. Those steps would be:
1. Ask for user input, including wording to indicate how to run the command
in such a way to bypass the need to ask for user input
2. Wait for the user to answer, and proceeding with a default answer upon
timeout (minimum: 30 seconds)
3. Do whatever is dictated by the answer, or timeout/default answer, as
appropriate
Waiting for a minimum timeout of 30 seconds here allows the user to read
what's going on and perform various checks to validate that they're in a good
position to continue. Checks like:
* Am I on wifi or did my laptop tether to my phone with it's highly
throttled/expensive connection? My wifi coverage has been spotty recently and
when it does my laptop "helpfully" likes to automatically switch to using my
phone's connection.
* How big is the diff between my branch and `main`? If it's too big, I may
not want to pull in changes yet. If it's tiny, then yes, pull in changes and
run again.
* Is the Docker daemon running? Oh right, I disabled it from automatically
starting on my old MacBook because it spins up the fans, but that means I have
to manually start it and I haven't done that yet. Gotta do that, and Docker
takes a bit to start on my old laptop.
* Wait, I just pulled in changes it shouldn't be asking about this! Oh wait,
whoops, no, I wanted to run this command in my other terminal, in my other
Airflow repository.
Every item of that mental checklist can take time, and it's very presumptive
that everybody running Breeze can tick off everything in that checklist within
a short timeout (eg: 10 seconds). Even 30 seconds is still majorly presumptive,
which is why I said that it should be minimum.
> Since you are proposing a change, I suggest you come up with and
experiment and gathering some data to prove your opinion is justified as I
think we have just different opinion on the subject.
If that's what it's going to take to make this change, then I'm just going
to give up and disengage from this discussion. That would be a very large
amount of effort to change this. I highly doubt that the original author of
this code went through such a study when selecting the timeout values, so I
think it's a bit unfair to ask somebody to go through so much effort to change
something that is (if I have adequately explained it here) pretty obvious and
non-subtle IMHO.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
