Skip to content

Conversation

@coderabbitai
Copy link

@coderabbitai coderabbitai bot commented Dec 6, 2025

Docstrings generation was requested by @lineardevon.

The following files were modified:

  • cmd/main.go
  • internal/config/config.go
  • internal/trino/client.go
  • internal/trino/external_auth.go
These files were ignored
  • internal/config/config_test.go
  • internal/trino/client_reauth_test.go
  • internal/trino/external_auth_test.go
These file types are not supported
  • CLAUDE.md
  • README.md
  • docs/deployment.md
ℹ️ Note

CodeRabbit cannot perform edits on its own pull requests yet.

Docstrings generation was requested by @lineardevon.

* #136 (comment)

The following files were modified:

* `cmd/main.go`
* `internal/config/config.go`
* `internal/trino/client.go`
* `internal/trino/external_auth.go`
@coderabbitai
Copy link
Author

coderabbitai bot commented Dec 6, 2025

Important

Review skipped

CodeRabbit bot authored PR detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Identified one logic issue in the new external-auth flow related to TLS handling; see inline comment.

return &ExternalAuthenticator{
baseURL: baseURL,
username: username,
httpClient: &http.Client{Timeout: 30 * time.Second},
Copy link

Choose a reason for hiding this comment

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

⚠️ Logic This HTTP client always uses the default TLS verifier, so TRINO_SSL_INSECURE / custom CA settings are ignored during the OAuth handshake. Clusters with self-signed certs cannot complete external auth; please build this client with the same TLS configuration as the main Trino connection or propagate the SSLInsecure flag.

@lineardevon
Copy link

Sorry, thought this would be a commit in the original PR where it was requested. Please close.

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.

2 participants