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

A set of device list cleanups #397

Merged
merged 4 commits into from
May 9, 2024
Merged

A set of device list cleanups #397

merged 4 commits into from
May 9, 2024

Conversation

vkhoroz
Copy link
Member

@vkhoroz vkhoroz commented Apr 19, 2024

No description provided.

This allows to fully move an argument parsing logic from the client into the command.
Before the change, an argument parsing logic was split between the two.

Signed-off-by: Volodymyr Khoroz <[email protected]>
A new API has a newer name argument for few months already.
Use it instead of the obsolete name_ilike.

Signed-off-by: Volodymyr Khoroz <[email protected]>
Currently, we onlhy special-cased the URL escaping of a ? for device name pattern.
But, other characters are not escaped, and uuid pattern is not escaped at all.

Most of the time this just works, but may produce unexpected results.
For example `fioctl devices list 'a*&'` will ignore the trailing ampersand,
and behave just like `fioctl device list 'a*'`, matching unexpected devices.

Using the `net/url` to URL escape the query makes filtering more predictable for such edge cases.

Signed-off-by: Volodymyr Khoroz <[email protected]>
This shortens the URL, making it easier to debug.

Signed-off-by: Volodymyr Khoroz <[email protected]>
@vkhoroz vkhoroz requested review from doanac and mike-sul April 19, 2024 17:10
@vkhoroz vkhoroz self-assigned this Apr 19, 2024
@vkhoroz vkhoroz merged commit 933d213 into main May 9, 2024
8 checks passed
@vkhoroz vkhoroz deleted the vkhoroz-devices-list-cleanup branch May 9, 2024 20:04
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.

3 participants