How would I access options defined on a GoalSubsystem subclass from logic implemented in a Subsystem class (specifically the one we use as a streaming_workunits_handlers) ? I just need the option values, not the instance of the GoalSubsystem subclass itself. @hundreds-father-404 @hundreds-breakfast-49010

That is, he wants to access the values of a

GoalSubsystem

in the method of a

Subsystem

, not in a rule Normally, we’d do this with

MySubsystem.global_instance()

, but `global_instance isn't defined on

GoalSubsystem

and I’m unclear if it should be? Any ideas? CC @witty-crayon-22786

i didn't realize a

GoalSubsystem

wouldn't be a

Subsystem

?

No, it implements

Optionable

and

SubsystemClientMixin

but not

Subsystem

. I’m not sure why that is, though?

does git blame say anything useful?

https://github.com/pantsbuild/pants/pull/8742/files#diff-dfe88b677a11becadc29a8affbbc9f70R16 not really. it seem to start as a goal not as a subsystem.

FYI this is the PR that first added the idea of a goal subsystem, then called `Goal.Options`: https://github.com/pantsbuild/pants/pull/6880

Because it wasn't a subystem before it was renamed... It was a Goal

But the inner class

Goal.Options

inherited

SubsystemClientMixin

and

Optionable

, rather than

Subsystem

. Do you remember why that is?

I don't have any particular concerns with having it extend Subsystem, but you'd want to look at that.

for run_tracker, we pass it all the options from

local_pants_runner

https://github.com/pantsbuild/pants/blob/master/src/python/pants/bin/local_pants_runner.py#L270 maybe we should just do the same for stream handlers ?

For code that is sufficiently close to the core, it can be fine to access options directly, yea.

@hundreds-breakfast-49010 ^^

But... It's a bit confusing to access the options of a Goal here. What's the usecase?

in our case, the

GoalSubsystem

subsystem implements the logic (and rules) to handle auth with our servers (OAuth flow, storing access tokens, etc...) so our stream handlers implementation needs to access the options to figure out where the access token is stored in order to load & use it.

Why not move that to a Subsystem?

I think for namespacing. We want to be able to do

./pants auth --foo

rather than

./pants --auth-subsystem-foo

It may not be specific to the goal if you're trying to use it elsewhere

I am not against that... I don't understand the semantics enough to have an opinion. @hundreds-father-404 wdyt ? should just make the auth options a regular subsystem ? can it then be consumed by other subsystems (and goals/rules) ?

Yes, if you make it a normal subsystem, then it could be used by other subsystems without needing any changes to Pants. Otherwise, we need to likely change Pants so that

GoalSubsystem

inherits

Subsystem

(which possibly makes sense - would need to audit more) The main downside I see is the namespace, as pointed out above.

"auth" as a verb is one thing, but it sounds like there are options independent of that verb.

yeah, if an option can be read outside of the

auth

goal, maybe that's a sign that it shouldn't be namespaced under

auth

Some of the options we have defined are: -

--base-url

-

--local-port

-

--output

(enum type) -

--from-env-var

Conceptually, I think it does make sense to have these options live in the goal, just as we have

./pants login --to

as an option. They modify how the goal runs. This goal is pretty simple - it doesn’t wrap any other tool like

Flake8

. It’s convenient and natural to be able to do

./pants auth --output=console

, as opposed to

./pants --authentication-output=console auth

-- The only subsystem names I could come up with are

AuthSettings

and

Authentication

, which is confusing why that’s distinct from the

AuthOptions(GoalSubsystem)

. That is, there’s no “natural” subsystem that comes to my mind, unlike the

Flake8

,

PythonSetup

, and

ScalaFix

subsystems, which all map to something very naturally.

@hundreds-father-404 so maybe that's an argument that

GoalSubsystem

should be a normal

Subsystem

that you can call

global_instance

on

Yes, I think that

GoalSubsystem

should inherit

Subsystem

. I can’t think of a strong conceptual motivation why we would not want that. (We should, though, still keep

GoalSubsystem

as a distinct class so that we can distinguish between normal subsystems. A

Goal.subsystem_cls

should only allow

GoalSubsystem

instances, not general

Subsystem

instances)

@hundreds-father-404: the options from https://pantsbuild.slack.com/archives/C046T6T9U/p1579222502123800?thread_ts=1579220301.118200&cid=C046T6T9U look like repo wide defaults rather than goal specific config

Indeed, they normally are defined in

pants.ini

under the scope

auth

So then it shouldn't matter whether they are on a goal or not?

goals implicitly introduce namespacing that might be desirable

then it shouldn’t matter whether they are on a goal or not?

What matters is the namespace. We want the options to live under the namespace

auth

(the same namespace as the goal). It’s confusing for us to have options defined on some subsystem called

Authentication

, so that you have to do

--authentication-local-port

rather than

--auth-local-port

. There’s no compelling reason for a new options scope, other than the constraints of Pants

the alternative is to invent a pseudo-namespace by just carefully naming the top-level rules

This seems related to the question of whether @rules should be able to depend on goal options... came up in the context of test coverage

if an @rule needs them, then they should be exposed as requestable types, like what I did for GlobalOptions a while ago

It feels like there are compositional/circular concerns

I agree that these are related. I see no reason that normal

@rules

should not be able to request a

GoalSubsystem

why's that? options can conceptually be RootRules, right? since before engine execution pants should have access to those options?

.global_instance is the v1 way... In a purely v2 run it will fail, I think

It feels like there are compositional/circular concerns

This would be true with

Goal.Options

, but now `GoalSubsystem`s can be used without ever needing to touch the

Goal

. They’re still closely coupled, but decoupled enough that it’s safe for a normal rule to consume the

GoalSubsystem

without any circular dependency on the

Goal

hm, global_instance isn't safe to use in a v2 rule?

hm, global_instance isn’t safe to use in a v2 rule?

No, you should request the subsystem in your rule signature and that subsystem should be registered via a

subsystem_rule()

@hundreds-father-404: one class or two is not what made them coupled... Moreso the conceptual connection

Conceptually, I view it as the

GoalSubsystem

being able to stand on its own, whereas the

Goal

needs the

GoalSubsystem

to work. For example, I think it does make sense to have in

pants.ini

options defined on your goal, like

[dependencies] transitive: True

because that’s the default you decide you want for your organization. It makes sense to have that option configured without running

./pants dependencies

. That is, the goal options conceptually can stand on their own.

Which would be fine if you were actually using them (only) in the goal: but you're not