Skip to content

Adding support for semantic layer apis - #36

Open
aniket-s-kulkarni wants to merge 3 commits into
dremio:mainfrom
aniket-s-kulkarni:semantic-layer-support
Open

Adding support for semantic layer apis#36
aniket-s-kulkarni wants to merge 3 commits into
dremio:mainfrom
aniket-s-kulkarni:semantic-layer-support

Conversation

@aniket-s-kulkarni

Copy link
Copy Markdown
Collaborator

No description provided.

@aniket-s-kulkarni
aniket-s-kulkarni marked this pull request as ready for review August 7, 2026 14:09

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31b2d3448d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/drs/cli.py
reflection,
role,
schema,
semantic,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Include the semantic command module before importing it

Every invocation of the installed dremio entry point imports this name, but a repo-wide search of the committed tree shows no src/drs/commands/semantic.py or generated equivalent. Importing drs.cli therefore raises an ImportError before any command can run; add the command module to this commit or remove its registration.

Useful? React with 👍 / 👎.

Comment thread src/drs/output.py
from enum import StrEnum
from typing import Any

import pandas as pd

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Declare pandas before importing it at startup

In a clean installation built from this commit, pandas is absent because it is declared in neither pyproject.toml nor uv.lock. Command modules import drs.output during CLI startup, so this unconditional import raises ModuleNotFoundError even when the user requests JSON output; either add the runtime dependency and update the lockfile or render the table using the already-declared Rich dependency.

Useful? React with 👍 / 👎.

Comment thread src/drs/commands/query.py Outdated
error("Cannot specify both a SQL argument and --file.")
raise typer.Exit(1)
sql = file.read_text().strip()
if file == '-':

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Compare the file option against a Path

When a user follows the option help and runs dremio query run --file -, Click converts the value to Path("-"), which is not equal to the string "-". The stdin branch is therefore skipped and file.read_text() attempts to open a literal file named -, usually raising FileNotFoundError; compare with Path("-") or otherwise detect the dash before reading.

Useful? React with 👍 / 👎.

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.

1 participant