Allow Tasks to include a Payload that determines their identity

Review Request #2009 - Created March 31, 2015 and updated

Information
Stu Hood
pants
1351
d492b90...
Reviewers
pants-reviews
areitz, davidt, jsirois, patricklaw
  • Add a Task lifecycle method that returns a Payload representing the unique identity of a Task

Since there are only two FingerprintStrategy subclasses, and they're both used to mix in Task options (ivy confs for IvyTaskMixin and library versions for JvmCompile), I think that it should be possible to replace/deprecate FingerprintStrategy with a mixing-in of Task identity.

If people think that sounds reasonable, I'll work on it on this review first thing tomorrow.

https://github.com/pantsbuild/pants/pull/1347

Issues

  • 1
  • 2
  • 0
  • 3
Description From Last Updated
These workdir names are going to make it difficult to poke around in .pants.d for debugging purposes. I like the ... Benjy Weinberger Benjy Weinberger
John Sirois
Patrick Lawson
Stu Hood
Review request changed

Bugs:

+1351
Benjy Weinberger

Before we go down this path, I'd like to step back a tiny bit and think about bringing some regularization/normality to the roughly overlapping concepts of "artifacts", "products" and "stuff in .pants.d". I'm hoping a design will fall out than unifies the common aspects of these around fingerprinting and invalidation.

src/python/pants/engine/round_engine.py (Diff revision 1)
 
 

These workdir names are going to make it difficult to poke around in .pants.d for debugging purposes. I like the idea of defaulting to a non-qualified name if the "identity payload" is trivial.

Stu Hood

Another fun anecdote to motivate a change like this:

Recently we bumped from scala 2.10.4 to 2.10.5... this correctly caused the artifact cache to miss, and everything seemed to have gone swimmingly. But because there was 2.10.4 analysis still sitting on disk, the next cache hits using the global strategy and pulling down 2.10.5 artifacts would merge it into the global analysis and get... 2.10.4 analysis (first header wins.) Result: zinc ignores the analysis and rewrites 2.10.5 analysis, which then gets merged back into the global analysis to make it invalid again.

Long story short, it took a week to notice that incremental compiles were disabled for anyone who hadn't run clean-all. Sure, we should fix the analysis merging. But there are no end of potential bugs that could be prevented by ensuring that tasks with different inputs are isolated from themselves.

  1. Another one: on https://rbcommons.com/s/twitter/r/2404/ we begin to build a per-task log. But since there isn't a non-destructive/standardized way to "swap" the configuration of a task, if you enable the flag while you have any existing state in pants.d, it's possible to get incomplete/partial logs. If the task-level configuration was included in a task identity, toggling the flag would give you a fresh location on disk, and preserve the invariants.

    As mentioned there, I'll probably ask Cody to work on this.

Loading...