I wonder if it makes sense to fail when we fail to...
# development
f
I wonder if it makes sense to fail when we fail to load AST of the sources due to the syntax error during dependency inference? We currently just silently proceed which leads to unpleasant surprises.
Without syntax errors:
Copy code
❯ ./pants dependencies helloworld/greet/greeting.py
//:reqs#setuptools
//:reqs#types-setuptools
helloworld/greet:translations
helloworld/translator/translator.py:lib
Adding a patch:
Copy code
diff --git a/helloworld/greet/greeting.py b/helloworld/greet/greeting.py
index 6a5eb1a..79d18f9 100644
--- a/helloworld/greet/greeting.py
+++ b/helloworld/greet/greeting.py
@@ -10,6 +10,7 @@ import pkg_resources
 
 from helloworld.translator.translator import LanguageTranslator
 
+this.is.a.problem
 
 class Greeter:
     def __init__(
...
Pants only suggests dependencies on
__init__.py
files and target (as per the BUILD file):
Copy code
❯ ./pants dependencies helloworld/greet/greeting.py 
helloworld/greet:translations
Is there a situation when you would want to proceed anyway ignore files with incorrect syntax? I've discovered this by accident when I noticed Pants wasn't reporting dependencies for a file that had many: I used
python.interpreter_constraints = ["==3.9.*", "==3.10.*"]
and Python 3.9 was used for dependency inference on a file that contained Python 3.10 syntax. I think this is a common situation enough (having mixed Python versions) so it perhaps deserves some thought.
e
@fresh-cat-90827 as an aside - you really want to avoid
["==3.9.*", "==3.10.*"]
, you should use
[">=3.9,<3.11"]
instead. The former is supported by Pants - sort of, but was never needed. The latter is Python-ecosystem standard and supported everywhere. Twitter added that non-standard-in-the-Python-ecosystem format when it was ignorant of
!=
range excludes FWICT back in the day.
🙏 1
h
I am excited to one day deprecate that list syntax
Although it may be impossible in practice...
But anyway, re failing on unparseable code, I can see both sides on that. Having the build system itself (rather than an underlying tool) fail on a syntax error in your code seems very aggressive.
On the other hand, it can lead to a more confusing error later
f
this makes sense, yes. The problem is not even an error that happens later - it's the fact that it's silently swallowing the error is what's concerning to me. I am happy with the flag -
--fail-on-parse
or something, but to give only 2 dependencies for a file that has 20 is a huge surprise 🙂
without any indication that the representation is likely to be incomplete