Skip to content

Fix: Exit if any config error - #15880

Open
cibinmathew wants to merge 2 commits into
python:masterfrom
cibinmathew:config-error
Open

Fix: Exit if any config error#15880
cibinmathew wants to merge 2 commits into
python:masterfrom
cibinmathew:config-error

Conversation

@cibinmathew

@cibinmathew cibinmathew commented Aug 15, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #15850

Few questions

  1. mypy_primer shows a lot of changes for deprecated mypy options like

https://github.com/dragonchain/dragonchain/blob/1a25eda598471fd735a2b515312bfc3bbe53770b/mypy.ini#L21
Should we ignore them.

  1. Should there be a check to exit inside this as well

    mypy/mypy/build.py

    Line 2210 in 76c16a4

    changes, config_errors = parse_mypy_comments(flags, self.options)
    See More

  2. Can this be ignored

    mypy/mypy/config_parser.py

    Lines 490 to 493 in 76c16a4

    if invert:
    print(f"{prefix}Can not invert non-boolean key {options_key}", file=stderr)
    continue
    try:

@cibinmathew
cibinmathew force-pushed the config-error branch 4 times, most recently from b58e5c7 to 98f8555 Compare August 15, 2023 23:08
@github-actions

This comment has been minimized.

Sign in to view
ikonst
ikonst reviewed Aug 15, 2023
View reviewed changes
Comment thread mypy/main.py Outdated
Comment on lines +1255 to +1256
stderr.write(f"{err}")
sys.exit(1)

@ikonst ikonst Aug 15, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
stderr.write(f"{err}")
sys.exit(1)
raise SystemExit(str(err))

@cibinmathew
cibinmathew force-pushed the config-error branch 2 times, most recently from ab49054 to c960d2a Compare August 16, 2023 23:13
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@cibinmathew
cibinmathew marked this pull request as ready for review October 3, 2023 21:34
@github-actions

github-actions Bot commented Oct 3, 2023

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

pybind11 (https://github.com/pybind/pybind11): typechecking got 113.40x faster (18.0s -> 0.2s)
(Performance measurements are based on a single noisy sample)
- setup.py:16: error: Skipping analyzing "setuptools.command.sdist": module is installed, but missing library stubs or py.typed marker  [import-untyped]
- setup.py:16: error: Skipping analyzing "setuptools.command": module is installed, but missing library stubs or py.typed marker  [import-untyped]
- setup.py:16: error: Skipping analyzing "setuptools": module is installed, but missing library stubs or py.typed marker  [import-untyped]
- setup.py:99: error: Class cannot subclass "sdist" (has type "Any")  [misc]
- pybind11/setup_helpers.py:66: error: Skipping analyzing "setuptools": module is installed, but missing library stubs or py.typed marker  [import-untyped]
- pybind11/setup_helpers.py:67: error: Skipping analyzing "setuptools.command.build_ext": module is installed, but missing library stubs or py.typed marker  [import-untyped]
- pybind11/setup_helpers.py:67: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
- pybind11/setup_helpers.py:69: error: Unused "type: ignore" comment  [unused-ignore]
- pybind11/setup_helpers.py:70: error: Unused "type: ignore" comment  [unused-ignore]
- pybind11/setup_helpers.py:87: error: Class cannot subclass "_Extension" (has type "Any")  [misc]
- pybind11/setup_helpers.py:269: error: Class cannot subclass "_build_ext" (has type "Any")  [misc]

dragonchain (https://github.com/dragonchain/dragonchain): typechecking got 169.71x faster (25.9s -> 0.2s)
(Performance measurements are based on a single noisy sample)
- dragonchain/lib/dto/eth_utest.py:51:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/eth_utest.py:72:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/eth_utest.py:77:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/eth_utest.py:88:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/eth_utest.py:115:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/eth_utest.py:123:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/eth_utest.py:131:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/interfaces/registry.py:28:5: error: Name "registry" already defined (by an import)  [no-redef]
- dragonchain/lib/dto/btc_utest.py:51:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:56:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:61:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:62:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:70:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:77:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:84:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:90:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:101:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:108:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:114:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:120:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:125:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:131:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/dto/btc_utest.py:138:9: error: Cannot assign to a method  [method-assign]
- dragonchain/lib/database/redis.py:399:36: error: Argument 2 to "zadd" of "SortedSetCommands" has incompatible type "Dict[str, int]"; expected "Mapping[Union[str, bytes], Union[bytes, float, int, str]]"  [arg-type]
- dragonchain/lib/interfaces/storage.py:42:5: error: Name "storage" already defined (by an import)  [no-redef]
- dragonchain/lib/database/redis_utest.py:32:30: error: Incompatible types in assignment (expression has type "None", variable has type "Redis[Any]")  [assignment]
- dragonchain/lib/database/redis_utest.py:38:34: error: Incompatible types in assignment (expression has type "None", variable has type "Redis[Any]")  [assignment]
- dragonchain/lib/database/redis_utest.py:46:9: error: "Callable[[str, int, int], Coroutine[Any, Any, Any]]" has no attribute "assert_called_once"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:61:42: error: Argument 2 to "zadd_async" has incompatible type "str"; expected "int"  [arg-type]
- dragonchain/lib/database/redis_utest.py:102:9: error: "Callable[[Union[str, bytes], Union[bytes, float, int, str], Union[None, float, timedelta], Union[None, float, timedelta], bool, bool, bool, bool, Optional[Any], Optional[Any]], Optional[bool]]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:106:9: error: "Callable[[Union[str, bytes], Union[bytes, float, int, str], Union[None, float, timedelta], Union[None, float, timedelta], bool, bool, bool, bool, Optional[Any], Optional[Any]], Optional[bool]]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:110:9: error: "Callable[[Union[str, bytes]], Optional[Any]]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:114:9: error: "Callable[[VarArg(Union[str, bytes])], int]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:122:9: error: "Callable[[bool, KwArg(Union[Any, Any])], bool]" has no attribute "assert_called_once"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:126:9: error: "Callable[[Union[str, bytes], VarArg(Union[str, bytes])], int]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:130:9: error: "Callable[[Union[bytes, float, int, str], VarArg(Union[bytes, float, int, str])], int]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:134:9: error: "Callable[[Union[bytes, float, int, str], VarArg(Union[bytes, float, int, str])], int]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:138:9: error: "Callable[[VarArg(Union[str, bytes])], int]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:142:9: error: overloaded function has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:146:9: error: overloaded function has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:150:9: error: "Callable[[Any, Any, Optional[int]], Any]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:154:9: error: "Callable[[Union[str, bytes]], Optional[Any]]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:158:9: error: "Callable[[Union[str, bytes], int], Optional[Any]]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:162:9: error: "Callable[[Union[str, bytes], Union[bytes, float, int, str], Union[None, float, timedelta], Union[None, float, timedelta], bool, bool, bool, bool, Optional[Any], Optional[Any]], Optional[bool]]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:166:9: error: "Callable[[Union[str, bytes], int, int], bool]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:170:9: error: "Callable[[Union[str, bytes], Union[str, bytes]], Optional[Any]]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:174:9: error: "Callable[[Union[str, bytes], VarArg(Union[bytes, float, int, str])], int]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:178:9: error: "Callable[[Union[str, bytes], Union[bytes, float, int, str]], bool]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:182:9: error: "Callable[[Union[str, bytes]], Set[Any]]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:186:9: error: "Callable[[Union[str, bytes], VarArg(Union[bytes, float, int, str])], int]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:190:9: error: "Callable[[Union[str, bytes], int, int], List[Any]]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:194:9: error: "Redis[Any]" has no attribute "pipelineassert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:198:9: error: "Callable[[Union[str, bytes]], int]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:202:9: error: "Callable[[Any, Any], Any]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:206:9: error: "Callable[[Any, Optional[int]], Any]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:210:9: error: "Callable[[Union[str, bytes]], Dict[Any, Any]]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:214:9: error: "Callable[[Union[str, bytes], Union[str, bytes]], bool]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/database/redis_utest.py:217:35: error: Argument 2 to "zadd_sync" has incompatible type "str"; expected "Dict[str, int]"  [arg-type]
- dragonchain/lib/database/redis_utest.py:218:9: error: "Callable[[Union[str, bytes], Mapping[Union[str, bytes], Union[bytes, float, int, str]], bool, bool, bool, bool, Optional[Any], Optional[Any]], int]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/error_reporter.py:37:5: error: Incompatible import of "reporter" (imported name has type "Callable[[str], None]", local name has type "Callable[[str, str], None]")  [assignment]
- dragonchain/lib/interfaces/storage_utest.py:47:9: error: Module has no attribute "cache_condition"  [attr-defined]
- dragonchain/lib/interfaces/storage_utest.py:55:9: error: "Callable[[str, str], bytes]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/interfaces/storage_utest.py:75:9: error: "Callable[[str, str, bytes], None]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/interfaces/storage_utest.py:83:9: error: "Callable[[str, Union[str, bytes], Optional[int], str], bool]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/interfaces/storage_utest.py:87:9: error: "Callable[[str, str], None]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/interfaces/storage_utest.py:95:9: error: "Callable[[str, str], int]" has no attribute "assert_called_once_with"  [attr-defined]
- dragonchain/lib/interfaces/storage_utest.py:108:9: error: "Callable[[str, str], bool]" has no attribute "assert_called_once_with"  [attr-defined]

... (truncated 145 lines) ...

@wyattscarpenter

wyattscarpenter commented Sep 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Not sure how useful it will be, especially at this chronological juncture, but here are my impressions and opinions:

Firstly: this is a good and useful thing to fix, in principle. Thanks for taking the time to work on it!

Your questions in the OP:

  1. Deprecated but valid configuration options should warn instead of error, and should not result in a positive status code or halt. (If they were already resulting in a positive status code, then we could continue doing that — but I don't think they were.)
  2. What's the alternative, that those configuration values aren't checked the same way? If so, then there should be a check there. They should be checked the same way. (Mypy comments are technically different than config files in various small ways, but we still want to complain if they're invalid.)
  3. If I understand what you're asking correctly: you can't ignore this; if someone tries to invert a flag that isn't invertible, this is invalid and a problem we should report.

Other:

  • I know there's some code in __main__ that does an exit 2 if something is wrong. I don't think that has to be changed at all in regards to this PR, but I thought I'd mention those in case you didn't know about them.
  • This PR should have tests associated with it. We already have some tests that assert the return value of runs like this, so it's a bit confusing to me that none of those needed to be changed. (Maybe we added those tests after 2023, who knows.)
  • The mypy primer diff is full of changes that seem wrong to me, like they shouldn't have resulted from this change, conceptually. Maybe that's something to look into. (Edit: oh, those lines are removals, because configuration errors now end the run before those errors can be reported. OK.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@ikonst ikonst ikonst left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Exit status when config error

3 participants

@cibinmathew @wyattscarpenter @ikonst