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.

Sign up for GitHub

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

Jump to bottom

Distinguish between Python vs JVM tool lockfile requests #16443

Closed
Eric-Arellano opened this issue Aug 8, 2022 · 0 comments · Fixed by #16449
Closed

Distinguish between Python vs JVM tool lockfile requests #16443

Eric-Arellano opened this issue Aug 8, 2022 · 0 comments · Fixed by #16449
Assignees
@Eric-Arellano

Comments

@Eric-Arellano
Copy link
Contributor

See #16420 (comment).

The best idea I have so far is to define subclasses for GenerateToolLockfileSentinel, like GeneratePythonToolLockfileSentinel. Then, we can check issubclass()... :/

A more elegant solution might be if we could inspect the rule graph to see if the rule returns the type GeneratePythonLockfile. I don't think that's currently possible though.

pants/src/python/pants/backend/python/lint/black/subsystem.py

Lines 108 to 114 in add0956

@rule(
desc="Determine Black interpreter constraints (for lockfile generation)",
level=LogLevel.DEBUG,
)
async def setup_black_lockfile(
_: BlackLockfileSentinel, black: Black, python_setup: PythonSetup
) -> GeneratePythonLockfile:

@Eric-Arellano Eric-Arellano self-assigned this Aug 8, 2022
This was referenced Aug 8, 2022
Merged
Merged
Eric-Arellano added a commit that referenced this issue Aug 9, 2022
@Eric-Arellano
GenerateToolLockfileSentinel encouraged to use language-specific su…
28d756e 
…bclasses (#16449)

Closes #16443, which is necessary for the per-resolve config project from https://docs.google.com/document/d/1HAvpSNvNAHreFfvTAXavZGka-A3WWvPuH0sMjGUCo48/edit# to work properly. See #16420 (comment). 

As discussed on the ticket, this is not a very elegant solution. But we don't have the rule graph introspection mechanism necessary to automagically figure out what type each tool is, and that also seems brittle.

As explained in this PR's updated docs, plugin authors don't have to technically make this change.

[ci skip-rust]
[ci skip-build-wheels]
cczona pushed a commit to cczona/pants that referenced this issue Sep 1, 2022
@Eric-Arellano @cczona
GenerateToolLockfileSentinel encouraged to use language-specific su…
4fda875 
…bclasses (pantsbuild#16449)

Closes pantsbuild#16443, which is necessary for the per-resolve config project from https://docs.google.com/document/d/1HAvpSNvNAHreFfvTAXavZGka-A3WWvPuH0sMjGUCo48/edit# to work properly. See pantsbuild#16420 (comment). 

As discussed on the ticket, this is not a very elegant solution. But we don't have the rule graph introspection mechanism necessary to automagically figure out what type each tool is, and that also seems brittle.

As explained in this PR's updated docs, plugin authors don't have to technically make this change.

[ci skip-rust]
[ci skip-build-wheels]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Eric-Arellano Eric-Arellano

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

GenerateToolLockfileSentinel encouraged to use language-specific subclasses Eric-Arellano/pants
1 participant
@Eric-Arellano