Skip to content

Change type annotations to match returned types, fixes #35#38

Merged
aniket-s-kulkarni merged 4 commits into
dremio:mainfrom
serraict:main
Jul 21, 2025
Merged

Change type annotations to match returned types, fixes #35#38
aniket-s-kulkarni merged 4 commits into
dremio:mainfrom
serraict:main

Conversation

@serra
Copy link
Copy Markdown
Contributor

@serra serra commented Jul 15, 2025

Fixes #35 Pydantic output validation errors for tools GetUsefulSystemTableNames and GetSchemaOfTable

serra added 2 commits July 15, 2025 16:02
Fixes dremio#35: Pydantic output validation errors for tools GetUsefulSystemTableNames and GetSchemaOfTable
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jul 15, 2025

CLA assistant check
All committers have signed the CLA.

@serra
Copy link
Copy Markdown
Contributor Author

serra commented Jul 16, 2025

I have some improvements coming for this today.

@serra
Copy link
Copy Markdown
Contributor Author

serra commented Jul 16, 2025

8f0fce1 is good to go.

Comment thread pytest.ini
python_classes = Test*
python_functions = test_*
asyncio_mode = strict
asyncio_default_fixture_loop_scope = function
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@serra - why do we need this for the test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I set it beacause of this warning I got when running the test:

(dremioai) ➜  dremio-mcp git:(main) ✗ uv run pytest
/Users/marijn/dev/serraict/dremio-mcp/.venv/lib/python3.12/site-packages/pytest_asyncio/plugin.py:217: PytestDeprecationWarning: The configuration option "asyncio_default_fixture_loop_scope" is unset.
The event loop scope for asynchronous fixtures will default to the fixture caching scope. Future versions of pytest-asyncio will default the loop scope for asynchronous fixtures to function scope. Set the default fixture loop scope explicitly in order to avoid unexpected behavior in the future. Valid fixture loop scopes are: "function", "class", "module", "package", "session"

  warnings.warn(PytestDeprecationWarning(_DEFAULT_FIXTURE_LOOP_SCOPE_UNSET))
=================================================================================== test session starts ====================================================================================

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

But tbh I did not read into the details any further, so I'm open to any other suggestions.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@@ -0,0 +1,61 @@
#
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@serra I have a PR lined up for a larger change to the unit tests to validate the output types. Perhaps we can skip this test and I will follow up with that PR.

Copy link
Copy Markdown
Contributor

@aniket-s-kulkarni aniket-s-kulkarni left a comment

Choose a reason for hiding this comment

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

I will do a follow up PR with more of the tests mocked.

@aniket-s-kulkarni aniket-s-kulkarni merged commit e1745d8 into dremio:main Jul 21, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Pydantic output validation errors for tools GetUsefulSystemTableNames and GetSchemaOfTable

3 participants