Repository navigation
fix: allow empty string as password - #825
Merged
Merged
Conversation
Clarify password requirements when username is provided.
mochnaufals
approved these changes
Dec 11, 2025
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical and moderate issues affect test execution, credential handling, generic typing, and CI enforcement.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
This pull request allows empty-string passwords and modernizes typing, tooling, and test fakes.
Changes:
- Distinguishes
Nonecredentials from empty passwords. - Adds and refines SDK type stubs and annotations.
- Updates Pixi type-checking configuration and fake collections.
| File | Summary |
|---|---|
tests/test_PIAF.py |
Removes an obsolete type ignore. |
tests/fakes.py |
Updates fake attribute collections; critical issue remains with FakeKeyValue iteration causing a TypeError (3 votes). |
pyproject.toml |
Adds type-checking configuration; moderate issue: ty_check is not gated by CI (2 votes). |
PIconnect/Time.py |
Adds explicit type aliases. |
PIconnect/Search.py |
Clarifies loop variable naming. |
PIconnect/PI.py |
Supports empty passwords; moderate issue with empty usernames bypassing supplied credentials (1 vote), plus a regression-test nit (4 votes). |
PIconnect/dotnet.py |
Improves SDK loading typing and errors. |
PIconnect/Data.py |
Refines pandas typing. |
PIconnect/Asset.py |
Clarifies optional PI point returns. |
PIconnect/AF.py |
Refines context-manager typing. |
PIconnect/_version.py |
Excludes generated version code from type-checking errors. |
PIconnect/_typing/System/Collections/Generic.py |
Adds generic collection protocols; moderate variance issue requires invariant type variables (1 vote). |
PIconnect/_typing/System/Collections/__init__.py |
Exports generic collection types. |
PIconnect/_typing/System/__init__.py |
Exposes collection mocks. |
PIconnect/_typing/AF/PI.py |
Adds explicit missing-value return typing. |
PIconnect/_typing/AF/EventFrame.py |
Uses Self for parent typing. |
PIconnect/_typing/AF/Asset.py |
Uses Self for parent typing. |
PIconnect/_typing/AF/_values.py |
Adds initializer return typing. |
PIconnect/_typing/AF/__init__.py |
Refines mock optionality. |
PIconnect/_typing/__init__.py |
Adds typing suppressions and protocol refinements. |
PIconnect/_collections.py |
Removes obsolete type ignores. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+85
to
+87
| self.attributes = Generic_.PropertyDict( | ||
| [FakeKeyValue(*att) for att in attributes.items()] | ||
| ) |
Comment on lines
+60
to
61
| ty_check = "ty check ." | ||
| format = { cmd = "ruff format --check .", depends-on = ["lint"] } |
| tuple[dotnet.System.Net.NetworkCredential, dotnet.AF.PI.PIAuthenticationMode] | ||
| | None | ||
| ) | ||
| if (username is not None) != (password is not None): |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #824