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

remove require_cuda defaults in internal functions #23

Merged
merged 1 commit into from
May 7, 2024

Conversation

jameslamb
Copy link
Member

Contributes to rapidsai/build-planning#31.

This project attempts to determine the version of CUDA present on the system using nvcc. By default, if that attempt fails it raises an exception.

This is configurable from the public API of the build backend, like this in pyproject.toml...

[tool.rapids-build-backend]
require-cuda = true

| `require-cuda` | If false, builds will succeed even if nvcc is not available | bool | true | Y |

This PR proposes removing defaults in the signatures of internal functions that perform implement that behavior, to reduce the risk of "oops forgot to pass this through and rapids-build-backend silently fell back to some a default" types of bugs.

In my opinion, it'd be useful to restrict knowledge of default values for configuration to exactly one place... the code used to parse configuration.

# Mapping from config option to default value (None indicates that option is
# required) and whether it may be overridden by an environment variable or a config
# setting.
config_options: "config_options_type" = {
"build-backend": (None, False),
"commit-file": ("", False),
"dependencies-file": ("dependencies.yaml", True),
"disable-cuda-suffix": (False, True),
"matrix-entry": ("", True),
"require-cuda": (True, True),
"requires": (lambda: [], False),
}

And in general, for there to be as few competing sources of configuration values as possible.

How I tested this

Looked for all uses like this.

_get_cuda

Otherwise, relying on this project's pre-commit + CI.

@jameslamb jameslamb added improvement Improves an existing functionality non-breaking Introduces a non-breaking change labels May 7, 2024
@jameslamb jameslamb requested a review from a team as a code owner May 7, 2024 18:26
@jameslamb jameslamb merged commit bbe5b13 into rapidsai:main May 7, 2024
3 checks passed
@jameslamb jameslamb deleted the stricter-cuda-config branch May 7, 2024 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improves an existing functionality non-breaking Introduces a non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants