Repository navigation
fix(abstractions): pass mypy --strict in store and fix the backing store bugs it shows - #800
Open
Max Azatian (HardMax71) wants to merge 3 commits into
Open
Max Azatian (HardMax71) wants to merge 3 commits into
Max Azatian (HardMax71) wants to merge 3 commits into
Conversation
…ore bugs it shows Annotate the rest of kiota_abstractions.store and default the BackingStore and InMemoryBackingStore type parameters to object, like RequestAdapter and ParsableFactory, so generated models that declare a bare `backing_store: BackingStore` still pass mypy --strict. BackingStoreFactorySingleton was never a singleton. `hasattr(cls, "__instance")` checks the unmangled name, so every call built a new instance and assigned the factory over the `backing_store_factory` property on the class, and a call with None put back a fresh InMemoryBackingStoreFactory. Generated models call it with None at import, so a factory passed to enable_backing_store never stuck. It now keeps one instance, None keeps the current factory, and a factory replaces it. InMemoryBackingStore stored lists as a (list, size) tuple and get() unwrapped any tuple, so a stored tuple such as (1, 2) came back as 1. Entries now keep the value and the list's size side by side, so get() returns what was set. enumerate_() still returns the (list, size) pair for lists. Subscribers now get the previous and new values, as subscribe documents, instead of the internal entries. Part of microsoft#722.
3 of 11 tasks
Contributor
Author
Wonder why I see it in PR diff, and in CI SonarCloud is green.. Anyways, it is outside of this PR scope, so ignoring it |
|
This branch has not been deployed
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.
Overview
Fourth item in #722.
kiota_abstractions.storenow passesmypy --strict, and errors inkiota_abstractionsgo from 42 to 20.BackingStoreandInMemoryBackingStorenow default their type parameter toobject, likeRequestAdapterin #787. So thebacking_store: BackingStore = field(...)line in generated models passes strict without regenerating. On main it has two errors.Adding types showed three bugs:
BackingStoreFactorySingletonwas never a singleton.hasattr(cls, "__instance")looks for a name that never exists, since Python renames__instanceinside the class. So every call made a new instance, and a call withNonereset the factory. Generated models make that call at import, so a factory passed toenable_backing_storenever stuck. Now there's one instance.Nonekeeps the factory and a factory replaces it, likeInstancein kiota-dotnet.InMemoryBackingStore.getunwrapped any tuple, so a stored(1, 2)came back as1. A list's size is now kept apart from the value, sogetreturns what was set.(False, "a"). They now get the old and new values, assubscribesays.Related Issue
Part of #722.
Notes
The singleton and subscriber fixes change runtime behavior. Models imported before
enable_backing_storekeep their old factory, since generated code binds it at import.Two smaller store bugs are fixed separately in #802: list items get one more subscription each time the list is set, and the
is_initialization_completedsetter writes back a stale list size.Testing Instructions
mypy --strict kiota_abstractionsinpackages/abstractionsshows nothing understore/, also with--python-version 3.10.mypy kiota_abstractions,mypy validationintests/validation,yapf -dr kiota_abstractionsandpytestpass.