Skip to content

feat: refactor cli parsing#1200

Open
Shubhranshu153 wants to merge 3 commits intorunfinch:mainfrom
Shubhranshu153:refactor-remote-parsing
Open

feat: refactor cli parsing#1200
Shubhranshu153 wants to merge 3 commits intorunfinch:mainfrom
Shubhranshu153:refactor-remote-parsing

Conversation

@Shubhranshu153
Copy link
Member

@Shubhranshu153 Shubhranshu153 commented Nov 29, 2024

Issue #, if available:

Description of changes:
For macos and windows and OS specific command prefix is attached to execute in the vm plane. As there is a vm plane, environment variables and credentials needs to be passed from the host to the vm. The limactl plane doesnt automatically handle this [Issue: https://github.com/lima-vm/lima/issues/1419]. To address this, the current finch handler used to parse or env flags [‘-e’,‘—env’,‘—env-file’], consolidated it and added it. For windows, the host path need to be translated to equivalent mount paths in the wsl environment.

In the proposed design, For macOS and windows we parse all env flags and add those env variables in the prefix of the limactl command. Here we dont modify the ordering or the contents of the command args reducing the complexity and chances of error getting introduced.

For windows path we do an inplace substitution based on regex pattern search across the args. This involves modifying the command args as windows to wsl path translation support for limactl shell execution is not provided. We use regexp to do a pattern match in substring of args split by separators. To validate this, we would rely on unit test and e2e tests.

Testing done:

  1. Automated Tests

TODO:
Devcontainers qualification tests.

  • I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Shubhranshu153 Shubhranshu153 force-pushed the refactor-remote-parsing branch 20 times, most recently from 9f211cf to 5ea6b14 Compare December 1, 2024 11:32
@Shubhranshu153 Shubhranshu153 reopened this Dec 1, 2024
@Shubhranshu153 Shubhranshu153 force-pushed the refactor-remote-parsing branch 8 times, most recently from d5b2a85 to 181583b Compare December 1, 2024 23:56
@Shubhranshu153 Shubhranshu153 force-pushed the refactor-remote-parsing branch 12 times, most recently from 79011b7 to 3f21783 Compare December 6, 2024 17:58
@Shubhranshu153 Shubhranshu153 force-pushed the refactor-remote-parsing branch 4 times, most recently from a471cac to f52ab39 Compare December 15, 2024 04:38
@Shubhranshu153 Shubhranshu153 marked this pull request as ready for review December 30, 2024 16:11
pendo324
pendo324 previously approved these changes Jan 21, 2025
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: can we move these create* functions to their own file or package?

Copy link
Contributor

Choose a reason for hiding this comment

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

All of these functions are only called once each. Are they here just for easier unit testing?

Signed-off-by: Shubharanshu Mahapatra <shubhum@amazon.com>
@Shubhranshu153 Shubhranshu153 force-pushed the refactor-remote-parsing branch from 962bf43 to 0884086 Compare December 17, 2025 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants