-
Notifications
You must be signed in to change notification settings - Fork 7
docs: Autogenerate list of connector capabilities for docs generation #466
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
""" WalkthroughA new Go program was introduced to scan connector directories, extract capability information using the registry package, and output the aggregated data as JSON. A corresponding Justfile recipe was added to build and run this program, and the pre-commit alias was updated to include this new compilation step. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Just as Justfile
participant GoTool as compile-capabilities (Go Program)
participant FS as Filesystem
Dev->>Just: Run pre-commit alias
Just->>GoTool: Execute compile-connector-capabilities recipe
GoTool->>FS: Scan connector directories
loop For each connector
GoTool->>FS: Read capabilities.go
GoTool->>FS: Parse AST, extract capabilities
end
GoTool->>FS: Write JSON output
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #466 +/- ##
=======================================
Coverage 69.66% 69.66%
=======================================
Files 627 627
Lines 32196 32196
==================
8000
=====================
Hits 22428 22428
Misses 8554 8554
Partials 1214 1214 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tools/compile-capabilities/main.go (4)
21-23
: Rename flag variable to avoid shadowing package namesThe
path
identifier shadows the standardpath
package (even if it’s not currently imported). Renaming it to something likeinputDir
orconnectorsDir
removes the potential for confusion and accidental misuse later.
53-57
: Emit deterministic, human-readable JSONUsing
json.Marshal
produces compact output and a map with random key order.
For version-controlled artefacts, indentation plus stable ordering makes diffs far clearer.- d, err := json.Marshal(&connectorCapabilities) + // Sort keys for deterministic output + keys := maps.Keys(connectorCapabilities) // Go 1.21 + slices.Sort(keys) + ordered := make(map[string][]string, len(keys)) + for _, k := range keys { + ordered[k] = connectorCapabilities[k] + } + d, err := json.MarshalIndent(ordered, "", " ")Requires
import "golang.org/x/exp/maps"
andimport "golang.org/x/exp/slices"
(or hand-rolled sorting).
81-83
: Usefilepath.Rel
for path validation instead ofHasPrefix
Prefix checks are fragile on case-insensitive filesystems or when symbolic links are involved.
filepath.Rel
(orfilepath.EvalSymlinks
) gives a safer guarantee that the target is inside the intended directory.
103-109
: Strip theCapability
prefix as well for cleaner JSON valuesCurrently you get
"CapabilityTransferToWallet"
after trimming"models."
.
If the docs table is meant to show the raw capability names ("TransferToWallet"
), extend the trim:- capabilities = append(capabilities, strings.TrimPrefix(str, "models.")) + capabilities = append(capabilities, + strings.TrimPrefix(strings.TrimPrefix(str, "models."), "Capability"))(Or use
strings.TrimPrefix(str, "models.Capability")
once.)Justfile (1)
24-27
: Run the tool withgo run
to avoid the build-cleanup danceCompiling a throw-away binary then
rm
-ing it adds I/O and complexity.
Consider:compile-connector-capabilities: @go run {{justfile_directory()}}/tools/compile-capabilities \ --path {{justfile_directory()}}/internal/connectors/plugins/public \ --output {{justfile_directory()}}/docs/other/connector-capabilities.json
Cleaner, faster, and nothing to delete.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
docs/other/connector-capabilities.json
is excluded by!**/*.json
📒 Files selected for processing (2)
Justfile
(2 hunks)tools/compile-capabilities/main.go
(1 hunks)
tools/compile-capabilities/main.go
Outdated
entries, err := os.ReadDir(*path) | ||
10000 | if err != nil { | |
log.Fatal(err) | ||
} | ||
|
||
connectorCapabilities = make(map[string][]string) | ||
for _, e := range entries { | ||
if !e.IsDir() { | ||
continue | ||
} | ||
|
||
capabilities, err := readCapabilities(e.Name()) | ||
if err != nil { | ||
log.Fatal(err) | ||
} | ||
|
||
connectorCapabilities[e.Name()] = capabilities |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Gracefully ignore connectors lacking capabilities.go
instead of aborting the run
readCapabilities
returns an error for every directory that does not contain a capabilities.go
file, and this error bubbles up to a log.Fatal
, terminating the whole program.
That will block the tool as soon as you add a connector that doesn’t implement capabilities yet.
A minimal change:
- capabilities, err := readCapabilities(e.Name())
- if err != nil {
- log.Fatal(err)
- }
+ capabilities, err := readCapabilities(e.Name())
+ if errors.Is(err, os.ErrNotExist) {
+ // Connector has no capabilities file – perfectly valid, just skip.
+ continue
+ }
+ if err != nil {
+ log.Fatalf("parsing capabilities for %s: %v", e.Name(), err)
+ }
(Remember to import "errors"
at the top.)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
entries, err := os.ReadDir(*path) | |
if err != nil { | |
log.Fatal(err) | |
} | |
connectorCapabilities = make(map[string][]string) | |
for _, e := range entries { | |
if !e.IsDir() { | |
continue | |
} | |
capabilities, err := readCapabilities(e.Name()) | |
if err != nil { | |
log.Fatal(err) | |
} | |
connectorCapabilities[e.Name()] = capabilities | |
entries, err := os.ReadDir(*path) | |
if err != nil { | |
log.Fatal(err) | |
} | |
connectorCapabilities = make(map[string][]string) | |
for _, e := range entries { | |
if !e.IsDir() { | |
continue | |
} | |
capabilities, err := readCapabilities(e.Name()) | |
if errors.Is(err, os.ErrNotExist) { | |
// Connector has no capabilities file – perfectly valid, just skip. | |
continue | |
} | |
if err != nil { | |
log.Fatalf("parsing capabilities for %s: %v", e.Name(), err) | |
} | |
connectorCapabilities[e.Name()] = capabilities |
🤖 Prompt for AI Agents
In tools/compile-capabilities/main.go around lines 34 to 50, the code currently
calls log.Fatal on any error returned by readCapabilities, which causes the
program to exit if a directory lacks a capabilities.go file. To fix this, modify
the error handling to check if the error indicates a missing capabilities.go
file (using errors.Is or a similar check) and if so, skip that directory without
logging a fatal error. Only call log.Fatal for other unexpected errors. This
change will allow the program to continue running even if some connectors do not
implement capabilities yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tools/compile-capabilities/main.go (1)
41-49
: Abort-on-missing-plugin logic blocks the entire run
registry.GetCapabilities
returnsErrPluginNotFound
when a connector hasn’t registered capabilities yet, but the code still callslog.Fatal
, terminating the tool.
Silently skipping those connectors (or at least degrading to a warning) keeps the generator useful while new connectors are under active development.- capabilities, err := registry.GetCapabilities(e.Name()) - if err != nil { - log.Fatal(err) - } + capabilities, err := registry.GetCapabilities(e.Name()) + if errors.Is(err, registry.ErrPluginNotFound) { // skip silently + continue + } + if err != nil { + log.Fatalf("reading capabilities for %s: %v", e.Name(), err) + }(Remember to add
import "errors"
and exportErrPluginNotFound
from the registry package if it is not already.)
🧹 Nitpick comments (1)
tools/compile-capabilities/main.go (1)
54-57
: Consider deterministic & human-readable JSON output
json.Marshal
produces non-stable key ordering and no indentation, which makes diffs noisy and the generated file hard to eyeball. Usingjson.MarshalIndent
on amap[string][]string
sorted beforehand (or converting to a struct with a deterministic order) yields cleaner PRs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tools/compile-capabilities/main.go
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tools/compile-capabilities/main.go (2)
internal/connectors/plugins/registry/plugins.go (1)
GetCapabilities
(122-130)internal/models/capabilities.go (1)
Capability
(9-9)
🪛 GitHub Actions: Default
tools/compile-capabilities/main.go
[warning] 80-80: Go lint warning: func astString
is unused
Relates to: formancehq/docs#126
This file is used to generate the following table in docs:
