Skip to content
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

Extract functionality to detect if the CLI is running on DBR #1889

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pietern
Copy link
Contributor

@pietern pietern commented Nov 7, 2024

Changes

Whether or not the CLI is running on DBR can be detected once and stored in the command's context.

By storing it in the context, it can easily be mocked for testing.

This builds on the simpler approach and conversation in #1744. It unblocks testing of the DBR-specific paths while not compromising on the checks we can perform to test if the CLI is running on DBR.

Tests

  • Unit tests for the new dbr package
  • New unit test for the ConfigureWSFS mutator

Copy link

github-actions bot commented Nov 7, 2024

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 1889
  • Commit SHA: 1ab0a59ba5cf26e9f3c5903adaddf6d6d8d42ad5

Checks will be approved automatically on success.

@eng-dev-ecosystem-bot
Copy link
Collaborator

Test Details: go/deco-tests/11731384827

Copy link
Contributor

@shreyas-goenka shreyas-goenka left a comment

Choose a reason for hiding this comment

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

Thanks! Minor comments.


import "context"

// key is a private type to prevent collisions with other packages.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I don't understand this comment. Wouldn't the type still be namespaced within the DB package?


// RunsOnRuntime returns the cached detection result from the context.
// It expects a context returned by [DetectRuntime] or [MockRuntime].
func RunsOnRuntime(ctx context.Context) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a one line note regarding why we cache the result? I presume it's because the stat("/databricks") call can be expensive on DBR.

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