Skip to content

Add --base-url flag for connector testability - #8

Merged
robert-chiniquy merged 4 commits into
mainfrom
rch/testability/fix-base-url
May 1, 2026
Merged

Add --base-url flag for connector testability#8
robert-chiniquy merged 4 commits into
mainfrom
rch/testability/fix-base-url

Conversation

@robert-chiniquy

Copy link
Copy Markdown
Contributor

Summary

Adds the --base-url CLI flag to enable overriding the default API endpoint for testing purposes.

Changes

  • Added BaseURLField to configuration
  • Updated client/connector to accept and use the base URL override
  • When --base-url is provided, it takes precedence over the default API URL

Files Modified

cmd/baton-discord/config.go,cmd/baton-discord/main.go,pkg/config/conf.gen.go,pkg/config/config.go,pkg/config/gen/gen.go,pkg/connector/connector.go

Testing

The connector can now be tested against mock servers:

baton-discord --base-url http://localhost:8080 [other-flags]

Related

Part of the Connector Testability initiative.

This change adds a --base-url CLI flag to allow overriding the default API
endpoint for testing purposes. When provided, the connector will use this
URL instead of the hardcoded production API URL.

This is part of the Connector Testability initiative to enable mock server
testing without modifying connector code.

Files changed: cmd/baton-discord/config.go,cmd/baton-discord/main.go,pkg/config/conf.gen.go,pkg/config/config.go,pkg/config/gen/gen.go,pkg/connector/connector.go
@robert-chiniquy
robert-chiniquy requested a review from a team February 8, 2026 07:51
@coderabbitai

coderabbitai Bot commented Feb 8, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@robert-chiniquy has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 2 minutes and 36 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch rch/testability/fix-base-url

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

@russellhaering russellhaering 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.

LGTM - base-url flag implementation looks correct. The discordgo library uses package-level endpoint globals, so the approach of setting them before session creation is the right pattern. Good modernization of the config package structure.

base-url is a dev/testing concern, not user-facing configuration.
Mark it WithHidden(true) so it doesn't appear in the hosted UI.

Good feedback from Geoff:
ConductorOne/baton-trayai#63 (comment)
@github-actions

github-actions Bot commented Apr 10, 2026

Copy link
Copy Markdown

Claude finished @robert-chiniquy's task in 1m 52s —— View job


PR Review: Add --base-url flag for connector testability

  • Gather PR diff and read changed files
  • Analyze code changes for correctness, security, and best practices
  • Post review feedback

Overall: Looks good. The approach of overriding discordgo's package-level endpoint globals is the correct pattern given that library's design. The config modernization is clean. Two reviewers have already approved.

I verified that all non-function Endpoint* string variables derived from EndpointAPI or EndpointDiscord are correctly reassigned in the override block. The function-based endpoints (e.g., EndpointGuild, EndpointUser, EndpointChannel) read the global vars at call time, so they pick up the new values automatically.

Minor observations

1. Unused DefaultBaseURL constant (pkg/connector/connector.go:17)

const DefaultBaseURL = "https://discord.com/"

This is declared but never referenced. It could be useful for documentation purposes, but if it's not going to be used (e.g., for validation or as a fallback), consider removing it to avoid dead code.

2. Pre-existing double-slash in EndpointVoice (pkg/connector/connector.go:74)

discordgo.EndpointVoice = discordgo.EndpointAPI + "/voice/"

This mirrors the original discordgo code (EndpointAPI + "/voice/") which produces .../api/v9//voice/ — a double-slash. This is not introduced by this PR (it's a quirk of the upstream library), just noting it for awareness.

3. Config improvements — Good call making token properly WithRequired(true) and removing the manual cobra flag registration. The WithHidden(true) and WithExportTarget(field.ExportTargetCLIOnly) on BaseURLField correctly restricts it to CLI-only use.

Summary

Clean implementation that correctly handles discordgo's global endpoint pattern. The config package restructuring is a nice modernization. No blocking issues.

@robert-chiniquy
robert-chiniquy merged commit 88911fc into main May 1, 2026
4 checks passed
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.

3 participants