Keyring Forger #22370
Draft
Keyring Forger #22370
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #22172
I opened this draft PR to demonstrate some of the cyclical import issues with setting up a keyring subsystem in Pants. Namely, the need to build a pex and execute keyring before executing a pex CLI process. The logic here is based on my understanding of what @cburroughs wanted to do with keyring injection. This method has the advantage of always running the keyring process locally and then making the result available within whichever execution environment the CLI process using keyring is running. Though I'm a little fuzzy on how to distinguish between which execution environment each process should run in so this may not be implemented correctly.
There may not be a way to actually due what I'm doing here with running imports within the
forge_keyringrule itself - the rule graph parsing currently fails for this module. I also am unaware of any syntax other than using aGetfor instantiating a subsystem like this, or if such a thing is even possible with the call-by-name changes.One alternative I had considered is just building the keyring binary with a recursive call within
setup_pex_cli_processand avoiding everything associated with the pex.py module, and not having a keyring subsystem. However, then the binary would not be exportable, or we'd have to duplicate a bunch of already existing logic to make it so.