search

superfly / flyctl

Command line tools for fly.io services
https://fly.io
Apache License 2.0
1.36k stars 224 forks source link

Bug fix: fly launch was not checking to see if directory name was unique #3637

Closed rubys closed 3 weeks ago

alichay commented 3 weeks ago

We can merge this to fix the bug, but I'm leaving this as more of a note to self than anything:

we have to validate the app name after the "plan" is constructed, which means that right now this will validate a good name twice. this makes it decently easy for normal usage to run against the rate limit for the name validation API.

rubys commented 3 weeks ago

I'll argue that it doesn't make sense to show a plan that is unlikely to succeed. This implies to me that we need to validate the name before we show the plan. Perhaps we only need to validate it a second time if it is changed by the launch UI?

This applies to other parts of the plan. I'm seeing a fair number of region-restricted fly launch failures; perhaps we shouldn't suggest a region unless it is available using the user's current plan?

alichay commented 3 weeks ago

I'll argue that it doesn't make sense to show a plan that is unlikely to succeed. This implies to me that we need to validate the name before we show the plan. Perhaps we only need to validate it a second time if it is changed by the launch UI?

no yeah I wasn't arguing whether this was right or not haha, it definitely is

I was just saying that it'll need to remember that the name's already been validated (if it has been) so it's not repeating itself, but that can come later :) thanks for getting this fixed, esp on a weekend!!