feat: add credentials validation for knowledge server

[mdesmet]

Important

Add credential validation to serve command in cli.py and update config loading logic in decorators.py.

• Behavior:
  • Add validate_credentials check in serve() in cli.py to abort if credentials are invalid.
  • Modify load_config_from_file() in decorators.py to return None instead of {} when config file is missing.
  • Adjust logic in auth_options in decorators.py to handle None from load_config_from_file().
• Functions:
  • Add validate_credentials import in cli.py for credential validation.
  • Update load_config_from_file() return type to Optional[Dict] in decorators.py.

^{This description was created by for 2b7121f. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 77457e5 in 1 minute and 30 seconds. Click for details.

• Reviewed 101 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/datapilot/cli/decorators.py:67

• Draft comment:
  Loading file config only when both token and instance_name are missing means a missing individual CLI option won’t be filled from the config file. Confirm if this change in merge behavior is intended.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. src/datapilot/cli/decorators.py:77

• Draft comment:
  Using 'or final_backend_url' may mask cases where altimateUrl is an empty string. Consider an explicit empty-string check.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% The comment raises a valid point about empty strings - using or will treat empty strings as falsy and fall back to the default URL. However, is this actually a problem? An empty string is not a valid URL, so falling back to the default URL in this case seems like reasonable behavior. The suggested change would make the code more explicit but doesn't fundamentally change the behavior in a meaningful way. The comment technically points out a real difference in behavior, but am I being too dismissive of the value of explicit checks? Maybe being more explicit would help future maintainers. While explicitness can be good, in this case the or operator provides clean, idiomatic Python code that handles invalid URLs (including empty strings) appropriately by falling back to the default. The comment should be deleted because the current code handles empty strings appropriately by falling back to the default URL, and the suggested change doesn't meaningfully improve the code.

3. src/datapilot/cli/decorators.py:68

• Draft comment:
  Typographic suggestion: In the comment "Try to Load configuration from file if no CLI arguments are provided", consider using lowercase "load" to maintain consistency with other comments (i.e., "Try to load configuration from file if no CLI arguments are provided").
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While there is a minor inconsistency in capitalization, this is an extremely trivial issue. The rules explicitly state not to make comments that are obvious or unimportant. Comment style/capitalization falls into this category - it doesn't affect code functionality or maintainability in any meaningful way. The comment is technically correct about the inconsistency. Perhaps maintaining consistent documentation style is more important than I'm giving it credit for? While consistency is good, this level of nitpicking about comment capitalization creates noise and distracts from more important code review feedback. The rules clearly state not to make obvious or unimportant comments. Delete this comment as it's too trivial and violates the rule about not making obvious or unimportant comments.

Workflow ID: wflow_703QlOTjOUA2AyOZ

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

For consistency, consider using err=True with click.echo for error messages (e.g., when credentials are invalid).

Suggested change

	click.echo("Error: Invalid credentials.")
	click.echo("Error: Invalid credentials.", err=True)

[suryaiyer95]

Looks ok. 1 comment

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed ff4afcb in 38 seconds. Click for details.

• Reviewed 14 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/datapilot/core/knowledge/cli.py:6

• Draft comment:
  Good change to absolute import for clarity. Verify that 'datapilot.clients.altimate.utils' is the correct path in all environments.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_hG9eV9xWFhquaHzK

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 2b7121f in 23 seconds. Click for details.

• Reviewed 13 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/datapilot/core/knowledge/cli.py:28

• Draft comment:
  Good improvement: adding err=True directs error output to stderr, matching the handling for missing credentials above.
• Reason this comment was not posted:
  Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_13Gl2M3d86bswpSw

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[suryaiyer95]

LG