search

pantsbuild / pants

The Pants Build System
https://www.pantsbuild.org
Apache License 2.0
3.33k stars 638 forks source link

Fix ca_certs_path to work with environments #16987

Closed Eric-Arellano closed 2 years ago

Eric-Arellano commented 2 years ago

We also need to fix the Twine option. It's not safe to use Path() here:

https://github.com/pantsbuild/pants/blob/7ecbf89a2240604a0bfae2aaca5d8b15c5aca7f7/src/python/pants/backend/python/subsystems/twine.py#L107

chrisjrn commented 2 years ago

Is this task to also address [GLOBAL].ca_certs_path, or just the way the Twine option is implemented?

Eric-Arellano commented 2 years ago

Yeah, also address [GLOBAL].ca_certs_path

chrisjrn commented 2 years ago

Re. the [GLOBAL] option, my initial understanding is that ca_certs_path is necessarily a bootstrap option as it's used to establish an REAPI connection at the scheduler level. Intrinsically, this must be an option value that belongs to the Pants process, and cannot be environment-sensitive. I suspect you'll want to continue using Path directly for the bootstrap value, knowing that it's always going to be used very early on in the bootstrap process.

Subsequently, the same option used in 99% of cases where we need to specify certs paths for use by subprocesses (twine is the exception here). This one absolutely makes sense as an environment-sensitive option.

As a straw-man proposal, we could split [GLOBAL].ca_certs_path into two options: [GLOBAL].ca_certs_path and [GLOBAL].subprocess_ca_certs_path.

ca_certs_path would remain a bootstrap option, and be used for bootstrapping the scheduler.

subprocess_ca_certs_path would be a global (subsystem) option that can be overridden per-environment. The default value for subprocess_ca_certs_path would be the value of ca_certs_path (offering a transparent upgrade path for most installations)

Accessing global_options.ca_certs_path directly should trigger a deprecation warning, directing users/plugin authors to use subprocess_ca_certs_path.