https://pantsbuild.org/ logo
Join Slack
Powered by
Does anyone know how to make sense of the rule gra...
# development
b
Does anyone know how to make sense of the rule graph errors from https://github.com/pantsbuild/pants/issues/21168#issuecomment-2660774751 ? The contributor (maybe @fresh-mechanic-68429? sorry if not the right one 😄) is attempting to use 
EnvironmentVarsRequest
but is blocked
h
Ah yes, the infamous meaningless rule graph errors… Ugh. Sorry about that @fresh-mechanic-68429. It is a huge downside of the graph solver that if you get a tiny thing wrong it spews out dozens of these errors that are really hard to track and make sense of.
f
👋 yeah thats me
👋 1
I'm somewhat familiar with fighting the rule graph, but the missing piece in my knowledge here is how the Environments feature works (presumably it needs to get the right environment to read env vars from)
Separately, theres the question of if this should even be using env vars for a specific environment, or if it should be restricted to the session (I think thats the right term for commands sent to the daemon?)
Since I couldn't get 
EnvironmentVarsRequest
working, I ended up finding 
SessionValues
and pulling the env vars like
Copy code
complete_env = session_values[CompleteEnvironmentVars]
aws_env_vars = {
    name: value
    for name, value in complete_env.get_or_match("AWS_*")
}
But wasn't sure if thats something recommended or not https://github.com/Zocdoc/pants/commit/f6c377b23b1adcf90c32cbb92941f4cd23e31e33#diff-9db7546d204ade92f3ca0eda531[…]92d675364c143d7b15fe0a8c42f674c35
h
IIRC I think EnvironmentVarsRequest has to be used with an “EnvironmentAware” subsystem in scope: https://www.pantsbuild.org/stable/docs/using-pants/environments#environment-aware-options
Actually, maybe not
Hmm
I don’t think you should access 
session_values
like that, but you should be able to request 
CompleteEnvironmentVars
as an argument to your @rule?
So if I change your rule’s signature to:
Copy code
@rule
async def download_from_s3(
    request: S3DownloadFile, complete_env: CompleteEnvironmentVars, global_options: GlobalOptions
) -> Digest:
That seems to work. Or at least the solver completes.
Hmm, this also solves for me:
Copy code
@rule
async def download_from_s3(
    request: S3DownloadFile, global_options: GlobalOptions
) -> Digest:
    from botocore import auth, compat, exceptions  # pants: no-infer-dep

    env = await Get(EnvironmentVars, EnvironmentVarsRequest(["AWS_FOO", "AWS_BAR"]))

    aws_credentials = await Get(
        AWSCredentials, 
        AWSCredentialsRequest(env_vars=env)
    )
Can you post a branch with the version that didn’t work for you? I can take a look
f
Heres a minimal reproduction with just the rule graph issue https://github.com/Zocdoc/pants/commit/b5fa2ab6edb7e14e17775ccd60f976a3a4a284b7 You'll need to add 
pants.backend.url_handlers.s3
to the pants.toml under 
backend_packages
otherwise that rule isn't actually registered (and the solver will complete)
Copy code
[GLOBAL]
pants_version = "2.24.1"
backend_packages = [
    "pants.backend.shell",
    "pants.backend.url_handlers.s3",
]
plugins = [
    'botocore==1.34.135'
]
fwiw using 
CompleteEnvironmentVars
throws the same rule graph error. I think your initial hunch was probably correct, "IIRC I think EnvironmentVarsRequest has to be used with an “EnvironmentAware” subsystem in scope" These are file targets, which don't have the EnvironmentField on them.
h
I don’t think so: there are plenty of call sites without that, and also adding a gratuitous arg of 
SystemBinariesSubsystem.EnvironmentAware
, just to try out the hypothesis, did not help
This feels like a solver bug, but I will dig further
It feels familiar, but I can’t quite recall why, so will have to do some debugging from first principles
So after some digging, this is not a solver error. The rule that produces a 
CompleteEnvironmentVars
requires an 
EnvironmentTarget
to be in scope. Usually one of those is provided by a target, but in this case there is no target in play. So the solver correctly fails.
But that said, in https://github.com/pantsbuild/pants/pull/21956 I see that you are bypassing direct access to env vars entirely, by letting botocore handle it? This means that Pants can’t invalidate if creds change, but I guess we wouldn’t actually want it to anyway, so this is probably the right way to go.
f
Thanks Benjy, I appreciate it. For 21956 (sigv4) I'm using the same pattern as the existing implementation, letting boto access the env vars directly, so I could address these issues separately. I still plan on attempting to fix the env var access (issues 21168, 20624) . Depending how people are setting up their local shell they may or may not have valid credentials when they issue a pants command the first time, so they expect to be able to set the credentials then re-issue the pants command. Currently that fails because the daemon is already running and the new env vars aren't forwarded to it. The url handler code is already ignoring the credentials regarding the cache, so I'd say its somewhat expected to not invalidate here.
so this is probably the right way to go.
You're referring to 
session_values[CompleteEnvironmentVars]
?
h
Ah no, I meant that bypassing the engine entirely and reading env vars directly in boto is the right way to go. But that was wrong of me due to the need to restart pantsd in such a case.
Which is annoying
I think we’ll want to force the “local” environment into context, let me see if that can be done
OK, yes, this is how:
You request 
local_environment_name: ChosenLocalEnvironmentName
as an arg to your rule
And you pass it through like this:
Copy code
env_vars = await Get(
        EnvironmentVars,
        {
            EnvironmentVarsRequest([
                "AWS_PROFILE",
                "AWS_REGION",
                "AWS_ACCESS_KEY_ID",
                "AWS_SECRET_ACCESS_KEY",
                "AWS_SESSION_TOKEN",
            ]): EnvironmentVarsRequest,
            local_environment_name.val: EnvironmentName,
        },
    )
That is the admittedly off-kilter syntax for passing multiple arguments to a 
Get
A dict of “value -> type that value satisfies”
f
Thanks Benjy, I put up the pr for this one https://github.com/pantsbuild/pants/pull/22008
3 Views
Open in Slack
PreviousNext
Powered by