Skip to content

feat: add find command for catalog search#174

Open
nadavs123 wants to merge 42 commits intomicrosoft:mainfrom
nadavs123:feature/catalog-search-find-command
Open

feat: add find command for catalog search#174
nadavs123 wants to merge 42 commits intomicrosoft:mainfrom
nadavs123:feature/catalog-search-find-command

Conversation

@nadavs123
Copy link
Copy Markdown

@nadavs123 nadavs123 commented Feb 15, 2026

Pull Request

Closes #172

Summary

Adds a new find command that searches for Fabric items across all accessible workspaces using the Catalog Search API (POST /v1/catalog/search).

Usage

# Basic search
fab find "sales report"

# Filter by item type
fab find "data" -P type=Lakehouse

# Multiple types (bracket syntax)
fab find "dashboard" -P type=[Report,SemanticModel]

# Exclude a type
fab find "data" -P type!=Dashboard

# Show detailed output with IDs
fab find "sales" -l

# Filter results client-side with JMESPath
fab find "sales" -q "[?type=='Report']"

# Project specific fields
fab find "data" -q "[].{name: name, workspace: workspace}"

Flags

Flag Description
-P/--params Filter in key=value or key!=value format. Brackets for multiple values: type=[Lakehouse,Notebook]
-l/--long Show detailed table with IDs (name, id, type, workspace, workspace_id, description)
-q/--query JMESPath query for client-side filtering

Implementation details

  • Follows existing CLI patterns: @fab_command decorator, print_output_format(), FabricCLIError
  • Item type list in type_supported.yaml (matches command_supported.yaml pattern)
  • Error messages in ErrorMessages.Find class (matches repo pattern)
  • Interactive mode: page-by-page with "Press any key to continue..." prompt
  • Commandline mode: fetches all pages automatically
  • _split_params handles bracket syntax and legacy comma syntax (type=Report,Lakehouse)
  • _fetch_results shared helper for response parsing
  • truncate_descriptions moved to fab_util.py for reuse

Files added

  • src/fabric_cli/client/fab_api_catalog.py — API client (search() function)
  • src/fabric_cli/commands/find/__init__.py — Package init
  • src/fabric_cli/commands/find/fab_find.py — Command logic
  • src/fabric_cli/commands/find/type_supported.yaml — Supported/unsupported item types
  • src/fabric_cli/errors/find.pyFindErrors class
  • src/fabric_cli/parsers/fab_find_parser.py — Argument parser
  • docs/commands/find.md — Command documentation
  • tests/test_commands/test_find.py — 39 unit tests + 5 e2e VCR tests
  • tests/test_commands/recordings/test_commands/test_find/*.yaml — VCR cassettes

Files modified

  • src/fabric_cli/core/fab_parser_setup.py — Register find parser
  • src/fabric_cli/errors/__init__.py — Register Find error class
  • src/fabric_cli/utils/fab_util.py — Add truncate_descriptions()
  • tests/test_commands/commands_parser.py — Register find parser + Windows PromptSession fix

Notes

  • Dashboard is the only unsupported item type (not searchable)
  • Dataflow Gen1/Gen2 are not searchable; only Dataflow Gen2 CI/CD is returned (as type Dataflow)
  • Scorecards are returned as type Report
  • Requires Catalog.Read.All scope (not yet added to CLI app registration)

Nadav Schachter added 21 commits February 9, 2026 17:51
Features:
- Search across all workspaces by displayName, workspaceName, or description
- Filter by item type with --type flag
- Limit results with --limit flag
- Detailed output with --detailed flag (includes id, workspaceId)
- Custom endpoint support with --endpoint flag or FAB_CATALOG_ENDPOINT env var

Output columns (default): name, type, workspace, description
Output columns (detailed): + workspaceId, id

Required scope: Catalog.Read.All
Unsupported types: Dashboard, Dataflow, Scorecard

Includes unit tests (12 tests passing)
…dling

Changes based on issue microsoft#172 feedback:
- Changed --type from comma-separated to nargs='+' (space-separated)
- Removed --endpoint flag (use internal mechanism instead)
- Added FabricCLIError for invalid/unsupported item types
- Added error handling for API failures
- Updated tests to match new patterns (15 tests passing)
- Added complete_item_types() completer for searchable types
- Tab completion excludes unsupported types (Dashboard, Dataflow, Scorecard)
- Restored unsupported type validation with clear error message
- Updated ALL_ITEM_TYPES list from official API spec
- Added SEARCHABLE_ITEM_TYPES for valid filter types
- 20 tests passing
- Keep tab-completion for --type flag
- Custom FabricCLIError for unsupported types (Dashboard, Dataflow, Scorecard)
- Custom FabricCLIError for unknown types
- Cleaner error messages vs argparse choices listing all 40+ types
- 22 tests passing
- Changed from data= to json= for request payload
- Added raw_response=True to avoid auto-pagination hanging
- Added fallback from displayName to name (API bug workaround)
- Updated tests to use dict instead of JSON string
- Successfully tested against dailyapi.fabric.microsoft.com
@nadavs123 nadavs123 requested a review from a team as a code owner February 15, 2026 09:54
Nadav Schachter and others added 7 commits February 24, 2026 16:25
…ination and -P params

- Interactive mode: pages 50 items at a time with 'Press any key to continue...'
- Command-line mode: fetches up to 1000 items in a single request
- Replace --type with -P type=Report,Lakehouse (key=value pattern)
- Remove --max-items and --next-token flags

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Catalog Search API returns an empty string continuationToken
on the last page instead of null/omitting it. This caused the
interactive pagination loop to send an empty token on the next
request, which the API treats as a fresh empty search — returning
unrelated results from the entire tenant.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Long descriptions caused the table separator line to wrap, appearing
as a double separator. Descriptions are now truncated with ellipsis
to keep the table within the terminal width. Full descriptions are
still available via -l/--long mode.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Command-line mode now fetches all pages instead of stopping at one
page of 1000. Uses the same continuation token pattern as interactive
mode with 'or None' guard against empty string tokens.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use contractions and active voice for friendlier tone
- Suggest close matches for unknown types instead of dumping all 43
- Remove redundant type list from unsupported type error

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
required=False,
metavar="",
nargs="*",
help="Parameters in key=value or key!=value format. Use brackets for multiple values: type=[Lakehouse,Notebook]. Use != to exclude: type!=Dashboard",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

If you parse it with get_dict_from_params, I think you will need to use quats: type=["Lakehouse","Notebook"]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Since I'm keeping a dedicated parser, no quotes are needed.

Copy link
Copy Markdown
Collaborator

@aviatco aviatco Mar 19, 2026

Choose a reason for hiding this comment

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

considering the option I suggest above, using get_dict_from_params, will required this change

assert payload["filter"] == "(Type ne 'Dashboard' and Type ne 'Datamart')"


class TestParseTypeParam:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you should create e2e tests that call find command and record the tests and validate all different scenarios instead of those unit tests.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done. Added 5 e2e tests with VCR cassettes covering basic search, type filter, long output, no results, and ne filter. I kept the unit tests for pure-function logic since those test edge cases and error paths that are hard to cover with e2e alone. Removed TestFindCommandline and TestFindInteractive.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think you should have only the e2e tests - and using that test all the different scenarios that will covered the unit tests.
@ayeshurun please keep me honest

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I converted the test suite to be e2e-first (16 e2e tests now) and removed every unit test that was redundant with an e2e equivalent. 13 remaining unit tests cover edge cases that can't practically be e2e.

@aviatco
Copy link
Copy Markdown
Collaborator

aviatco commented Mar 15, 2026

Please update the description:

fab find "monthly" --type Report Warehouse - should be list

fab find "dashboard" --max-items 10 - flag doesnt exist

fab find --next-token "eyJTa2lwIjoy..." - flag doesnt exist

nadavs123 and others added 8 commits March 15, 2026 17:11
This is just a draft of the issue I opened
Changes:
- Move type lists to type_supported.yaml (loaded at import)
- Create FindErrors class for error messages
- Rename catalog_search -> search in fab_api_catalog
- Extract _fetch_results helper (shared by interactive/commandline)
- Add try-except JSONDecodeError around json.loads
- Refactor while True -> while has_more
- Rename detailed -> show_details, _parse_type_param -> _parse_type_from_params
- Single-loop _display_items
- Move truncate_descriptions to fab_util.py
- Fix 'total item(s)' -> 'item(s) shown'
- Remove redundant comments
- Parser: remove required=False, change -P nargs to '?'
- Handle both string and list params (backward compat)
- _split_params respects brackets and merges legacy comma values
- Delete issue-172.md
- Flatten test dir: tests/test_commands/test_find.py (43 tests)
- Add docs/commands/find.md

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Register find parser in CLIExecutor (commands_parser.py)
- Add TestFindE2E class with 5 VCR-based tests:
  basic search, type filter, long output, no results, ne filter
- Cassettes need to be recorded with --record flag and
  FAB_TOKEN/FAB_API_ENDPOINT_FABRIC env vars

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The @singleton decorator replaces InteractiveCLI with a closure function,
so class-level attribute access (InteractiveCLI.init_session) fails. Instead,
patch PromptSession in the fab_interactive module to inject DummyInput/
DummyOutput before the singleton creates the PromptSession instance.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Mock builtins.input with EOFError to stop pagination after first page
- Normalize cassette URIs from dailyapi → api.fabric.microsoft.com
- Fix test_find_with_long_output: check 'ID:' not 'id'
- Fix test_find_with_ne_filter: check 'Type: Dashboard' not just 'Dashboard'

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…e2e)

These integration-style unit tests overlap with the VCR e2e tests.
Keep pure-function unit tests for _split_params, _build_search_payload,
_parse_type_from_params, _raise_on_error, and _display_items.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Align find -l with ls -l and acl patterns — both compact and long
modes now use show_headers=True for table rendering. Long mode columns:
name, id, type, workspace, workspace_id, description (full text).
Description column shown in both modes only when any item has one.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fab find 'data' -P type=Lakehouse
# search for multiple item types (bracket syntax)
fab find 'dashboard' -P type=[Report,SemanticModel]
Copy link
Copy Markdown
Collaborator

@aviatco aviatco Mar 19, 2026

Choose a reason for hiding this comment

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

should be updated once using get_dict_from_params - type=['Report','SemanticModel']

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I updated the docs to use get_dict_from_params. I kept the unquoted bracket syntax (for example, type=[Report,SemanticModel]) so it stays consistent with other commands like mkdir -P paths=[Files/reports,Files/datasets].

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The example should show a working command example - so if running find command with -P type=[Report,SemanticModel] (without quotes) is working I'm om with it

fab find 'finance' -P type=[Warehouse,Lakehouse] -l
# filter results client-side with JMESPath
fab find 'sales' -q "[?type=='Report']"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Isn’t this effectively the same as -P type='Report', but with poorer performance due to fetching all items before filtering? We might want to skip this example since it’s not a best practice.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I'm not even sure why we added -q in the first place, as all filters should be done server-side anyways imo... I added it as part of the initial review requests.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

-q is usefull for column filtering.
find command return the name, ws name. desc etc... if user want to see only the name, for example, user can use -q for that.

"""Search for nonexistent term shows 'No items found'."""
cli_executor.exec_command("find 'xyznonexistent98765zzz'")

# print_grey is used for "No items found." but it's not mocked here
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you have mock_print_grey you can use, but I do believe also in mock_questionary_print you can see the output. if you dont see it that means something is wrong with the test

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I updated the no-results test to assert through mock_print_grey that No items found. is printed.

mock_questionary_print,
):
"""Search with type!=Dashboard excludes Dashboard items."""
cli_executor.exec_command("find 'report' -P type!=Dashboard")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

we should also test with multiple values like: type!=[Report,Notebook] and verify those types didn't return in the result

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I added a test for multi-value negation with bracket syntax: type!=[Report,Notebook].

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From the test I can see it is working without the quotes... did you verify it in command_line as well? I dontmran here in the tests, I mean manually from the console

# ---------------------------------------------------------------------------


class TestFindE2E:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

missing failure scenarios where the API return an error, where the json parse failed, where the type is unknomn etc...
any error we have in the code should be tested.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I added failure scenario tests for API error responses, malformed JSON responses, and unknown type errors.

# ---------------------------------------------------------------------------


class TestFindE2E:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Relevant for all tests - the name should have prefix of failure or success - depend on the test. you can see examples in any other test's file

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I renamed all test methods to use _success or _failure suffixes.

Nadav Schachter and others added 3 commits March 19, 2026 23:13
…st improvements

- Replace _split_params with shared get_dict_from_params utility
- Update get_dict_from_params regex to support != operator
- Move unsupported_parameter/invalid_parameter_format to CommonErrors
- Remove unsearchable_type from FindErrors (use CommonErrors.type_not_supported)
- Use ERROR_UNEXPECTED_ERROR constant in _raise_on_error
- Extract _print_search_summary helper function
- Reorder has_more assignment in _find_interactive
- Remove JMESPath type filter example and Notes section from docs
- Remove TestSplitParams class and legacy comma-syntax tests
- Add e2e tests: case-insensitive type, multi-value !=, unknown type, unsupported type
- Rename all e2e tests with _success/_failure suffix
- Add _assert_strings_in_mock_calls helper for cleaner assertions
- Rename cassettes to match new test names, add 2 new cassettes

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add e2e tests: invalid param format, unsupported param, unsupported param !=, JMESPath query
- Remove redundant unit tests now covered by e2e (payload single type, ne single type, invalid format, unknown param, unknown param !=)
- Test split: 26 unit + 13 e2e = 39 tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add e2e: multi-type eq, JSON output, search summary with count
- Remove 14 unit tests now fully covered by e2e (DisplayItems, parse/payload overlaps)
- Remove unused imports and sample data constants
- Final split: 13 unit + 16 e2e = 29 tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
uri: https://api.fabric.microsoft.com/v1/catalog/search
response:
body:
string: '{"value": [{"id": "bb0df270-bbc3-4713-a3f3-47010daa20d5", "type": "KQLQueryset",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

When recording no private information should be visible in the recording.
Please use this guide: https://github.com/microsoft/fabric-cli/blob/main/tests/authoring_tests.md

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sanitized all cassettes with mock values from static_test_data.py



# Sample API responses for testing
SAMPLE_RESPONSE_WITH_RESULTS = {
Copy link
Copy Markdown
Collaborator

@aviatco aviatco Mar 24, 2026

Choose a reason for hiding this comment

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

we don't want to have a sample response - we should trigger the API and confirm the actual response.
In each test we should add a setup phase where we created/set/update the needed items them call find and rely on the items we created to verify they are in the results. also it's better to use 'mock' data and not actual values of items

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

See above responses on this issue.

- Pass max_depth=1 to get_dict_from_params (find uses flat keys only)
- Sanitize all VCR cassettes: replace real IDs, workspace names, display names with mock values
- Delete unused ErrorMessages.Config.invalid_parameter_format (now in Common)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nadavs123
Copy link
Copy Markdown
Author

nadavs123 commented Mar 24, 2026

@aviatco , while testing locally I noticed that print_output_format breaks the table layout when item names exceed the terminal width. I did a small patch for "description" but the issue also reproduces with long item names. I think a more robust fix would be terminal-width-aware column sizing in the shared table renderer (_print_output_format_result_text). I prefer not to do it as part of this PR as I don't know what other functions it might affect. Filed as #201 .

Nadav Schachter and others added 2 commits March 25, 2026 22:34
Support both current flat fields (workspaceName, workspaceId) and
upcoming nested format (hierarchy.workspace.displayName, .id).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace truncate_descriptions with truncate_columns: priority-based
  truncation (description -> workspace -> name), with absolute max
  width cap (50 chars) and terminal-width fitting
- Strip trailing spaces before ellipsis
- Show running total in interactive pagination instead of per-page count

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 29, 2026 12:05
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new top-level fab find command that searches Fabric items across all accessible workspaces via the Catalog Search API (POST /v1/catalog/search), integrating with existing CLI output/error patterns and adding unit + VCR-backed E2E coverage.

Changes:

  • Introduces find command implementation, parser registration, and supported-type allow/deny lists.
  • Adds a dedicated Catalog API client wrapper for catalog/search.
  • Extends shared utilities (get_dict_from_params to support !=, plus a new truncate_columns helper) and adds tests + VCR recordings + docs.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/fabric_cli/commands/find/fab_find.py Implements find command logic, paging, filtering, JMESPath support, and output formatting.
src/fabric_cli/commands/find/type_supported.yaml Declares searchable vs unsupported item types for catalog search.
src/fabric_cli/commands/find/__init__.py Adds find command package.
src/fabric_cli/client/fab_api_catalog.py Adds API client wrapper for POST /v1/catalog/search.
src/fabric_cli/parsers/fab_find_parser.py Adds argparse parser, flags, help, and examples for find.
src/fabric_cli/core/fab_parser_setup.py Registers find with the global parser setup.
src/fabric_cli/errors/find.py Adds FindErrors message helpers.
src/fabric_cli/errors/__init__.py Registers ErrorMessages.Find.
src/fabric_cli/errors/common.py Adds common error helpers used by param parsing (unsupported_parameter, invalid_parameter_format).
src/fabric_cli/errors/config.py Removes config-specific invalid_parameter_format helper (superseded by common).
src/fabric_cli/utils/fab_util.py Updates param parsing to accept != and adds truncate_columns() for table-fit truncation.
docs/commands/find.md Documents find usage, flags, and examples.
tests/test_commands/commands_parser.py Registers find parser in test CLI harness and adjusts Windows PromptSession handling.
tests/test_commands/test_find.py Adds unit tests + VCR-backed E2E tests for find behaviors.
tests/test_commands/recordings/test_commands/test_find/test_find_basic_search_success.yaml VCR cassette for basic search.
tests/test_commands/recordings/test_commands/test_find/test_find_with_type_filter_success.yaml VCR cassette for type= filtering.
tests/test_commands/recordings/test_commands/test_find/test_find_type_case_insensitive_success.yaml VCR cassette for case-insensitive type=.
tests/test_commands/recordings/test_commands/test_find/test_find_with_long_output_success.yaml VCR cassette for --long output.
tests/test_commands/recordings/test_commands/test_find/test_find_no_results_success.yaml VCR cassette for empty results.
tests/test_commands/recordings/test_commands/test_find/test_find_with_ne_filter_success.yaml VCR cassette for type!= filtering.
tests/test_commands/recordings/test_commands/test_find/test_find_ne_multi_type_success.yaml VCR cassette for type!=[...].
tests/test_commands/recordings/test_commands/test_find/test_find_multi_type_eq_success.yaml VCR cassette for type=[...].
tests/test_commands/recordings/test_commands/test_find/test_find_with_jmespath_query_success.yaml VCR cassette for -q/--query behavior.
tests/test_commands/recordings/test_commands/test_find/test_find_json_output_success.yaml VCR cassette for --output_format json.
tests/test_commands/recordings/test_commands/test_find/test_find_search_summary_success.yaml VCR cassette for summary output.
.changes/unreleased/added-20260209-171617.yaml Adds unreleased changelog entry for the new command.

Comment on lines 73 to 95
params_dict: dict = {}
# Split the params using a regular expression that matches a comma that is not inside a pair of quotes, brackets or braces
# Example key1.key2=hello,key2={"hello":"testing","bye":2},key3=[1,2,3],key4={"key5":"value5"}
# Result ['key1.key2=hello', 'key2={"hello":"testing","bye":2}', 'key3=[1,2,3]', 'key4={"key5":"value5"}']
# Example key1.key2=hello
# Result ['key1.key=hello']
pattern = r"((?:[\w\.]+=.+?)(?=(?:,\s*[\w\.]+=)|$))"
pattern = r"((?:[\w\.]+!?=.+?)(?=(?:,\s*[\w\.]+!?=)|$))"

if params:
if isinstance(params, list):
norm_params = " ".join(params)
else:
norm_params = params

# Remove from multiline
norm_params = norm_params.replace("\\", "").strip()

matches = re.findall(pattern, norm_params)
if not matches:
raise FabricCLIError(
ErrorMessages.Config.invalid_parameter_format(norm_params),
ErrorMessages.Common.invalid_parameter_format(norm_params),
fab_constant.ERROR_INVALID_INPUT,
)
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

The new parameter-regex allows key!=value by treating it as a key! key. Since this changes get_dict_from_params() behavior (and error text) across the CLI, it would help to document the != support directly here (docstring/comment), and consider whether downstream callers should be insulated from !-suffixed keys unless they explicitly opted into that behavior.

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +36
assert match_found, f"Expected strings {strings} to {'all be present together' if require_all_in_same_args else 'be present'} in mock calls, but not found."
else:
assert not match_found, f"Expected strings {strings} to {'not all be present together' if require_all_in_same_args else 'not be present'} in mock calls, but found."
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

This test file contains a couple of very long f-strings (e.g., the assertion messages in _assert_strings_in_mock_calls) that don't appear to be Black-formatted and may fail black --check in CI. Reformat these assertions (let Black wrap them) to keep the file compliant with the repo formatter.

Suggested change
assert match_found, f"Expected strings {strings} to {'all be present together' if require_all_in_same_args else 'be present'} in mock calls, but not found."
else:
assert not match_found, f"Expected strings {strings} to {'not all be present together' if require_all_in_same_args else 'not be present'} in mock calls, but found."
assert match_found, (
f"Expected strings {strings} to "
f"{'all be present together' if require_all_in_same_args else 'be present'} "
"in mock calls, but not found."
)
else:
assert not match_found, (
f"Expected strings {strings} to "
f"{'not all be present together' if require_all_in_same_args else 'not be present'} "
"in mock calls, but found."
)

Copilot uses AI. Check for mistakes.

try:
utils_ui.print_grey("")
input("Press any key to continue... (Ctrl+C to stop)")
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

The prompt says "Press any key to continue..." but the implementation uses input(...), which requires pressing Enter (and will echo a newline). Either adjust the prompt text to reflect this, or use an input mechanism that truly reads a single keypress (or a consistent fab_ui prompt helper) in interactive mode.

Suggested change
input("Press any key to continue... (Ctrl+C to stop)")
input("Press Enter to continue... (Ctrl+C to stop)")

Copilot uses AI. Check for mistakes.
Comment on lines +293 to +298
if not show_details:
utils.truncate_columns(display_items, ["description", "workspace", "name"])

if getattr(args, "query", None):
display_items = utils_jmespath.search(display_items, args.query)

Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

_display_items() truncates columns unconditionally for non---long output, which will also mutate the data returned in --output_format json (and can affect -q/--query results). Truncation should be applied only for text/table output (and ideally after any JMESPath filtering/projection), leaving JSON output unmodified for scripting.

Suggested change
if not show_details:
utils.truncate_columns(display_items, ["description", "workspace", "name"])
if getattr(args, "query", None):
display_items = utils_jmespath.search(display_items, args.query)
if getattr(args, "query", None):
display_items = utils_jmespath.search(display_items, args.query)
# Apply truncation only for non-detailed, non-JSON outputs, after any JMESPath filtering.
output_format = getattr(args, "output", None) or getattr(args, "output_format", None)
is_json_output = output_format == "json"
if not show_details and not is_json_output:
utils.truncate_columns(display_items, ["description", "workspace", "name"])

Copilot uses AI. Check for mistakes.
all_items.extend(items)
has_more = continuation_token is not None
if has_more:
payload = {"continuationToken": continuation_token}
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

When paging, the code replaces the request payload with only {"continuationToken": ...}. This drops pageSize (and any filter), which can change paging behavior (e.g., command-line mode may fall back to the API default page size on subsequent requests) and increases request count. Keep pageSize/filter consistent across pages (or explicitly include them alongside continuationToken) rather than relying on continuation token semantics.

Suggested change
payload = {"continuationToken": continuation_token}
payload["continuationToken"] = continuation_token

Copilot uses AI. Check for mistakes.
mock_questionary_print,
):
"""Search with type!=Dashboard excludes Dashboard items."""
cli_executor.exec_command("find 'report' -P type!=Dashboard")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From the test I can see it is working without the quotes... did you verify it in command_line as well? I dontmran here in the tests, I mean manually from the console

+ str(mock_fab_ui_print_error.call_args_list)
)
assert "Dashboard" in all_output
assert "not supported" in all_output
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

please use the message from ErrorMessage constants you used in the code - so in case the values will changes it will not affect the tests.
in other tests as well

"""Search with malformed -P value shows error."""
cli_executor.exec_command("find 'data' -P notakeyvalue")

all_output = (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why did you concat the prints values?
It is better to verify your expected error is in the correct steam - mock_fab_ui_print_error and even better to verify mock_questionary_print was not called.

same for other tests


mock_questionary_print.assert_called()
_assert_strings_in_mock_calls(
["Report"],
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

_assert_strings_in_mock_calls get "strings" as value.. why not passing both report and lakehouse while other values are identical. you can avoid calling it twice this why.

def test_returns_items_and_token(self, mock_search):
response = MagicMock()
response.status_code = 200
response.text = json.dumps(SAMPLE_RESPONSE_WITH_RESULTS)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

once you call find command you dont need those tests. you can verify the values in the command response

mock_print_grey,
):
"""Search with results prints summary with item count."""
cli_executor.exec_command("find 'data'")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

you can verify this in the other tests as part of the assertion avoiding having a specific test for it

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.

[FEATURE] Add find command for catalog search

3 participants