Skip to content

feat(recipe): deprecate SetDataProvider / GetDataProvider globals #987

Description

@mchmarny

Part of #983. Depends on #986.

Summary

Once per-Builder DataProvider isolation lands (#986), the package-global recipe.SetDataProvider and recipe.GetDataProvider are the only remaining process-global state in the recipe path. Mark them as deprecated and point new callers to WithDataProvider.

This is Stage 1 (godoc deprecation only). Removing the symbols is a follow-up that must wait for CLI / API server migration.

Motivation

The point of #986 is that recipe resolution no longer needs a global provider — every consumer can hold its own. Leaving SetDataProvider as a documented entry point invites new code to reproduce the multi-tenant hazard the rest of the cleanup eliminated. Deprecating the symbol communicates the intended direction without breaking any existing caller.

The CLI's initDataProvider at pkg/cli/root.go and any other call site that still uses SetDataProvider continue to work unchanged through Stage 1. They get migrated in Stage 2.

Proposed change

1. Godoc deprecation markers

Add the standard Go deprecation marker to both functions:

// SetDataProvider sets the global data provider.
//
// Deprecated: prefer recipe.NewBuilder(recipe.WithDataProvider(dp))
// to bind a provider to a specific Builder instance. The package-
// global provider is retained for back-compat with the CLI and API
// server but will be removed in a future release. See #983 for the
// migration plan.
func SetDataProvider(provider DataProvider) { ... }

// GetDataProvider returns the global data provider.
//
// Deprecated: callers that need a specific provider should hold their
// own reference (typically obtained from NewLayeredDataProvider or
// NewEmbeddedDataProvider) and pass it via WithDataProvider rather
// than relying on package state. See #983.
func GetDataProvider() DataProvider { ... }

The Go vet / staticcheck deprecation lint (SA1019) will surface these as warnings to anyone importing the symbols, including the AICR codebase itself — that's intentional, because it gives us a TODO list for Stage 2 migration.

2. Optional: lint-allowlist the existing in-tree call sites

If staticcheck's SA1019 is enabled in .golangci.yaml (or becomes enabled), the existing in-tree uses in pkg/cli/root.go and any test helpers will start failing CI. Either:

  • Add //nolint:staticcheck // tracked by #983 Stage 2 comments at the current call sites (preferred — keeps the TODO list visible), or
  • Leave SA1019 disabled until Stage 2 migrates the call sites.

Pick one in the PR and document the choice in the PR description.

3. Documentation

  • Update docs/contributor/data.md to lead with WithDataProvider as the primary pattern, demote SetDataProvider to a back-compat note.
  • Update the pkg/recipe package godoc.

Acceptance

  • Both functions carry the standard // Deprecated: godoc marker pointing readers to WithDataProvider.
  • go vet ./... clean.
  • The deprecation surfaces in IDE tooling (gopls) and staticcheck output where applicable.
  • docs/contributor/data.md updated.
  • No runtime behavior change. Existing call sites continue to work.

Out of scope

  • Stage 2 migration of in-tree call sites (pkg/cli/root.go and any other SetDataProvider callers). Separate follow-up issue gated on this one.
  • Removal of the symbols. That comes after Stage 2 has shipped in a release.

Dependencies

Metadata

Metadata

Assignees

No one assigned

    Fields

    No fields configured for Enhancement.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions