Skip to content

[v1][breaking] Tighten EvaluatorResponse interface (typed accessors, time.Duration, drop Get prefix) #86

@robbyt

Description

@robbyt

Summary

platform/evaluatorResponse.go:5-21:

type EvaluatorResponse interface {
    Type() data.Types
    Inspect() string
    Interface() any
    GetScriptExeID() string
    GetExecTime() string  // <-- returns Duration.String()
}

Three changes worth making before v1.

1. GetExecTime() stringExecTime() time.Duration

The duration is held internally as time.Duration and stringified at the boundary:

  • engines/extism/evaluator/response.go:80
  • engines/risor/evaluator/response.go:60

Stringifying durations at the boundary is anti-idiomatic.

2. Drop the Get prefix from getters per Go style

ScriptExeID(), ExecTime().

3. Consider typed accessors

Every integration test currently does result.Interface().(map[string]any) (~30 occurrences in engines/integration_test.go). Adding helpers like:

AsString() (string, bool)
AsMap()    (map[string]any, bool)
AsBool()   (bool, bool)
AsInt()    (int64, bool)
AsSlice()  ([]any, bool)

would remove a lot of ceremony. Open question: should these return a sentinel error instead of bool? Risor already exposes a typed object via risorObject.Object embedded in execResult, so a Risor path could be exact; the Extism path is any based on JSON.

Files

  • platform/evaluatorResponse.go
  • engines/extism/evaluator/response.go
  • engines/risor/evaluator/response.go
  • engines/starlark/evaluator/response.go

This is a breaking-change candidate for v1.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions