https://pantsbuild.org/ logo
Join Slack
Powered by
How does it work that I have pytest spec'd from a ...
# general
f
How does it work that I have pytest spec'd from a resolve at 7.3, but some code that does 
import pytest
and is resolved to pytest 7.0 ? I thought Pants/Pex would choke on that but it seems to be working stranglely
b
"First one wins" I believe... (or whatever the Pex behavior is)
I can't seem to find the issue, but there was/is one that documented this phenomenon
e
Pex behavior is Python behavior, 1st on 
sys.path
wins. IIRC Pants constructs a pytest runner PEX once using the pytest "tool" resolve and then joins that with the code under test PEX.
Speed hack.
f
I'm happy it works... but it seems fragile. Seems to be a consequence of there only being one possible tool resolve for pytest
e
Benjy added stuff to use the pytest from your resolve in new Pants.
b
Not in 2.16.x 😉 You'd point the 
pytest
resolve to your code's resolve
f
I am
but I have two resolves that both need pytest 😄
e
Clearly not?
Resolves != requirements declarations. 1 requirement decl can be a part of 2 resolves.
f
i have "pip-f37" resolve, which is mirroring what's available in Fedora 37 (pytest 7.1), and I have "pants-plugins" which is just a minimal resolve to get in repo plugins testable (7.0, to keep pants.testutil happy). Pytest is pointed at the "pip-f37" resolve:
Copy code
[pytest]
install_from_resolve = "pip-f37"
requirements = ["pytest"]
but it works for the pants-plugin resolve
e
Ah - gotcha.
f
That this worked is what suprised me
b
Hmmm @happy-kitchen-89482 how is tool resolves supposed to work with multiple-user-resolves?
f
fwiw I don't know why pants.testutil doesn't want pytest >= 7.1
e
In Pants v1 - for JVM - we used "shading" for this. Tools had their package names all re-written save for the main entrypoint and public types - like pytest.fixture / pytest.mark, etc.
So you could muddle through separating the runner from the API (junit). This worked well, but is a pretty complicated solution.
@flat-zoo-31952 you're winning because pytest is pretty stable despite its version numbers.
f
yeah I figured this would be fine
which for one thing means we could probably lift the version issue on pants.testutil
I can open a PR for that
b
Seems like the solution would be something like
Copy code
[pytest]
inherit_resolve = True
meaning the resolve for 
pytest
at build time should be the resolve of the target. Although, I'm sure we'll find out why we can't do that 😅
f
yeah
e
That sounds like the right thing to do to me.
f
it looks like it's a function of how 
test
is written, where it resolves pytest first, then resolves the local_dists, and then merges them
e
Not really, this is a Python issue. something must win. The test goal can't get around that.
f
I meant for the fix
h
It is in 2.16.x
you set 
install_from_resolve=name_of_resolve
on the tool's subsystem
f
I mean that it looks like the loaded lockfile for tool resolves only happens once per interpreter constraint https://github.com/pantsbuild/pants/blob/962caab13a7c90469f0354d166fd9dac950b282b/src/python/pants/backend/python/subsystems/python_tool_base.py#L144 based on the install_from_resolve value
b
Right, but what if you have multiple resolves for your code? You want pytest for each test to come from that tests resolves
h
Not sure where multiple-user-resolves come into it?
That is not possible today (nor was it possible before)
b
but it should be...
h
maybe
f
to support the putative "inherhit_resolve" you'd need to generate loaded locked pexes for each tool and thread the resolve name for each
I can take a look at adding this, but it seems a bit tricky
h
That would be a complex feature, and it's not just pytest
theoretically any tool could want this
I think it's probably easier to script around this?
b
pytest
is special because you use it as a tool AND import it
f
any tool that is meant to be imported
so mostly test tools
b
like you don't import 
black
and expect it to work when running 
black
h
Yes but you could still want different versions of black for different parts of your codebase
❗ 1
f
it could also happen with testing other tool plugins
h
But I think this can be scripted around?
b
Yeah fair @happy-kitchen-89482
h
I mean, this is not something we've supported in the past, and it's not generally how python tooling works
f
I can see someone building a mypy plugin needing to import mypy for multiple versions of mypy (another nasty example)
h
your codebase is usually expected to be compatible with one version of a tool
But for now I'd be interested to see how far you can get by setting 
--pytest-install-from-resolve=resolve
for different parts of the codebase?
f
my codebase is, the problem is i'm building pants-plugins 😄
which require python3.9 and pytest < 7.1
h
Probably we should loosen that pytest requirements in Pants itself...
I think it's spurious
b
FWIW Benjy I see this as "we migrated several repos into our monorepo, but each team still wants their own fiefdom"
➕ 1
f
yeah i'm looking at that
h
Yeah, but that is a huge design space to think about
It would require substantial thinking and designing
b
I've yet to implement the "partion-by-config" feature that would lend it to this as well
h
because what goes into a "fiefdom"?
f
yeah that's gonna vary a lot by org
h
right now we happen to have one use-case for pytest in one org
but...
b
mmmm I suspect there are many many cases of this that just aren't vocal 🙂
f
i mean my use case goes away if we unlock the pants.testutil pytest version
h
This comes back to a couple of related issues I've been wanting us to think about for a while: • Inheriting BUILD file metadata up the filesystem • The artificial split between config that goes in BUILD files vs pants.toml
👆🏻 1
👆 1
b
It could also be we don't hear about it because it's an excellent scapegoat for whoever maintains the build. Alternatively it could be "Pants doesn't support it, so we won't use pants"
h
Well, nothing else supports it either. So it's more like "we won't have a monorepo"
b
• Inheriting BUILD file metadata up the filesystem
Ask @curved-television-6568 how many times I've bugged him in the past few months regarding this 😉
f
in the wild there are lots of bad monorepos though
h
Basically, imagine if all config was in BUILD files, and you could override any of it
f
I think the reason I chose Pants was because it seemed the most amenable to adapting to that and slowly pushing towards better thigns
h
That would ~solve the problem at the design level
The implementation of that would be very, very non-trivial
b
Well, nothing else supports it either. So it's more like "we won't have a monorepo"
Makefiles wouldsolve this, if I may...
f
agree with the build BUILD thing but yeah that sounds like a lot of work
b
Anywho, we ramble...
c
@bitter-ability-32190 so much I’ve actually started to make some progress just the other day 😅
b
(and for the record, I'd love to see every 
PathGlob
be satisfied by a target, and never not from a target)
c
think I have a pretty basic novel approach that could be a good ground for a POC to discuss around… draft PR forthcoming eventually…
😈 1
f
for the record
Copy code
pants test pants-plugins:: --no-pytest-config-discovery --pytest-install-from-resolve=pants-plugins
works for now and I think that's a decent workaround
(i mean it worked before, but only by virtue of pytest having a stable API and us not digging through the unstable 
_pytest
parts of it in our tests)
c
• The artificial split between config that goes in BUILD files vs pants.toml
I’ve been playing with the idea in my head about having subsystems configured in BUILD files.. sounds pretty much like ☝️ That would then of course be possible to have different configs for the same subsystem in different parts of your repo. The “trick” then is to have the config propagate properly to each target that consumes them.
h
This would be a major redesign, whose time is nigh. I think it would have to go along with a rethink of what targets (and target types) mean, and how they get used in a context-dependent way. We keep butting up against that question. It fuels things like https://github.com/pantsbuild/pants/pull/19155 for example.
b
Yeah I think the next progression is "tagged dependencies". That PR you linked really lays the groundwork for filtered dep graphs.
h
I suspect a fresh look, rather than trying to graft this on to existing concepts, might be what we want.
This might be one of those times where a design doc is called for, rather than a PoC
b
Oh most defintely. I consider it an extension of my original "intuitive dependencies" proposal 🙂
But something like:
Copy code
some_target(
    dependencies=[
        dependency("<addr>", for_goals=["check"], asset_type="file")
    ]
)
where 
dependency
is an object with some known keys (and maybe plugin-provided keys as well) would make dep traversal much richer
f
Currently one of the big difference between 
pants.toml
and 
BUILD
files is that nothing really has 
pants.toml
as filedeps so for those of us that use the 
--changed
subsystem 
pants.toml
won't trigger things. This can be really useful at times (since touching a .BUILD dirties all of its targets from a 
--changed
perspective), but also sometimes things that should be checked get missed.
c
I would love for pants to be able to discern which targets where actually affected by a changed file, rahter than just “everything in it”
f
I guess this is a bit off topic, but it would be an interesting enhancement to the changed subsystem to have it dive deeper into .BUILD files
👍 1
c
applies for changes to requirements.txt as well…
and similar (for target generators)
f
I think you get cache hits if you have cache setup, which may be why this isn't noticed as much
There's some kinda fundamental difference between between using the changed subsystem to get performant builds vs using cache, and I think the cache is the prvililedged method. I'd like to be able to bring those more in line (but it may not be technically feasible in all cases)
b
Yeah --changed is an imperfect approximation. Good for local dev use, but not great for CI. I think we could make that more approximate, but I also think it'll never be 1:1, and that's ok
f
It would be really really nice to make it closer. I don't really have a path to using caching in CI in my current repo, as it would be too expensive and I'm way too worried about poisoning a distributed cache.
Also I don't even use Pants to actually run tests currently because of several reasons: 1. Our prod code uses RPM not PyPI 2. I don't have a docker friendly setup to support running in environments to deal with this. 3. Too many tests make assumptions that they're running from a source dir 4. Pantsd uses way too much memory that I need to actually run tests. (I need a 4 GB limit and some of our tests need all 16 GB that are on the machines we run them on). 5. Test suites are too long and Pants doesn't stream job, which is unnerving for anyone that ever watches the jobs So cache-based running is not currently an option for me. Even still, I get a lot of value out Pants. It's just the vision of a 
pants lint check test
CI simplicity doesn't feel realistic in anyway.
Anyways, just something to consider if we're talking about changes to how BUILD files work and how config works in general
2 Views
Open in Slack
PreviousNext
Powered by