local-dev: pull images on every run + add support for tty resizing#200
local-dev: pull images on every run + add support for tty resizing#200ayushkamat merged 6 commits intomainfrom
Conversation
Signed-off-by: Ayush Kamat <ayush@latch.bio>
Signed-off-by: Ayush Kamat <ayush@latch.bio>
Signed-off-by: Ayush Kamat <ayush@latch.bio>
Signed-off-by: Ayush Kamat <ayush@latch.bio>
|
|
||
| wf_name = "" | ||
| for entity in FlyteEntities.entities: | ||
| if isinstance(entity, PythonFunctionWorkflow): |
There was a problem hiding this comment.
what if multiple workflows are defined in the package
There was a problem hiding this comment.
they would all have the same version, so not an issue
| resp.raise_for_status() | ||
| latest_version = resp.json()["version"] | ||
|
|
||
| return f"{config.dkr_repo}/{current_workspace()}_{pkg_root.name}:{latest_version}" |
There was a problem hiding this comment.
we really need to not make the name of the workflow container depend on the name of the directory
think it should be more like git, where project directory name has nothing to do with repository name
There was a problem hiding this comment.
we can add it to the backlog lol
| async def send_message( | ||
| ws: websockets.WebSocketClientProtocol, | ||
| message: Union[str, bytes], | ||
| typ: Optional[str] = None, |
There was a problem hiding this comment.
making this into an optional enum would make things more legible, its unclear to me what values are allowed and therefore where things might break
| raise EOFError | ||
|
|
||
| if not COMMAND_EXPRESSION.match(cmd): | ||
| await aioconsole.aprint("Invalid command") |
There was a problem hiding this comment.
parsing out useful errors from a failed match would be instructive here (eg. run-scripts vs run-script often trips me up)
There was a problem hiding this comment.
also a good call
| continue | ||
|
|
||
| if ( | ||
| cmd.startswith("run") |
There was a problem hiding this comment.
you should actually be able to include arbitrary whitespace characters before your first command and still get desired behavior (try with bash)
| EXEC_COMMANDS = ["shell", "run", "run-script"] | ||
| LIST_COMMANDS = ["ls", "list-tasks"] | ||
|
|
||
| COMMAND_EXPRESSION = re.compile("^(run\s.+|run-script\s.+|shell|list-tasks|ls.*)$") |
There was a problem hiding this comment.
you should actually be able to include arbitrary whitespace characters before your first command and still get desired behavior (try with bash)
| resize_event_queue = asyncio.Queue() | ||
|
|
||
| def new_sigwinch_handler(signum, frame): | ||
| if isinstance(old_sigwinch_handler, Callable): |
There was a problem hiding this comment.
i also remember there is a builtin to do this - https://docs.python.org/3/library/functions.html#callable, but i actually can't think of why this is better
i'm pretty sure isinstance(obj, Callable) iff obj has __call__ which is exactly what builtin does, literally cant find anything else about it
There was a problem hiding this comment.
the cases where i dont get a callable arent things i need to handle - SIG_IGN is literally ignore the signal in this process, SIG_DFL for the signal im handling here (SIGWINCH) means the current handler does the default action for SIGWINCH, which is also to ignore it (https://www.gnu.org/software/libc/manual/html_node/Miscellaneous-Signals.html), and None ofc means that there is no handler for it
Signed-off-by: Ayush Kamat <ayush@latch.bio>
Signed-off-by: Ayush Kamat <ayush@latch.bio>
2 participants
No description provided.