index operator interface #165

ggpwnkthx · 2023-09-21T13:15:03Z

Small QOL update to the OperationIndex class that adds the __getitem__ method to enable access to operations via index operations (square brackets).

codecov · 2023-09-21T13:18:25Z

Codecov Report

Merging #165 (14bee50) into master (ec8a583) will increase coverage by 0.03%.
The diff coverage is 80.89%.

@@            Coverage Diff             @@
##           master     #165      +/-   ##
==========================================
+ Coverage   92.78%   92.82%   +0.03%     
==========================================
  Files          86       86              
  Lines        6639     6703      +64     
==========================================
+ Hits         6160     6222      +62     
- Misses        479      481       +2

Flag	Coverage Δ
unittests	`92.82% <80.89%> (+0.03%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
aiopenapi3/openapi.py	`92.42% <100.00%> (-0.05%)`	⬇️
aiopenapi3/request.py	`92.61% <100.00%> (+0.07%)`	⬆️
tests/path_test.py	`99.68% <100.00%> (+0.01%)`	⬆️
aiopenapi3/extra.py	`98.95% <95.83%> (-1.05%)`	⬇️
tests/extra_test.py	`70.27% <61.90%> (+6.41%)`	⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

commonism · 2023-09-22T07:13:05Z

Please consider setting up pre-commit, having the ci bot create merge commits for pre-commit changes does not allow rebaseing and requires cherry picking all commits in a new branch.

Anyway - have a look, I sneaked in some bit's I have had and added a unit test.

ggpwnkthx · 2023-09-25T15:15:01Z

I'm not familiar with pre-commit, but it looks simple to use. Is there anything I need to do beyond the following?

pip install pre-commit
pre-commit install
pre-commit run --all-files

It seems to get stuck on the flake8 step for me.

commonism · 2023-09-26T05:46:14Z

It does not allow committing non-compliant changes.
If it gets stuck, it requires changes, but it does these changes as well.
Use with git gui to create commits. If you commit only segments of a file, the auto correction does not allow applying the change, commit full file, have the file corrected in the reject, commit corrected file, change last commit using amend.

FAILED tests/extra_test.py::test_reduced[Reduce] - TypeError: 'NoneType' object is not iterable
FAILED tests/extra_test.py::test_reduced[Cull] - TypeError: 'NoneType' object is not iterable

Small QOL update to the OperationIndex class that adds the __getitem__ method to enable access to operations via index operations (square brackets).

If the key is None, instead of looking based on path + method it will search for the operationId. This is slightly slower than dict comprehension, but still quick and the code is a bit more readable. A path's non-operation objects are also kept. (cherry picked from commit 6b58d47)

commonism · 2023-09-26T06:09:29Z

Never merge, always rebase.
PyCharms git integration is really lovely, it allows to do these kind of operations graphical.

I rebased this to allow merging.

commonism · 2023-09-26T07:10:43Z

If you want to incooperate the use of operationIds in Reduce I think it'll be better to have the Reduce.operations a List instead of a dict.

List[
    Union[
        Tuple[
            Union[Pattern,str],
            Optional[List[str]]
        ], 
        str]
]

Using a dict will do as well, it's basically

{None:["operationIdA","operationIdB"], r'/test':["get"]}

vs

["operationIdA", "operationIdB", (r"/test", ["get"])]

where the use of a tuple or a str is similar to the parameters when creating a request.

ggpwnkthx · 2023-09-26T12:32:29Z

If you want to incooperate the use of operationIds in Reduce I think it'll be better to have the Reduce.operations a List instead of a dict.

I considered that as well as breaking it out into two separate arguments: paths and operations. I didn't like the separate argument because I felt the naming would cause confusion. I fell back to the single argument, because it would maintain the existing interfacing mechanism. I think the

List[
    Union[
        Tuple[
            Union[Pattern,str],
            Optional[
                List[
                    Union[Pattern,str]
                ]
            ]
        ], 
        str
    ]
]

is the way to go. Using a tuple also make sense, since the incoming filters should be immutable.

I'll update the PR today.

commonism · 2023-09-26T20:59:47Z

> List[
>     Union[
>         Tuple[
>             Union[Pattern,str],
>             Optional[
>                 List[
>                     Union[Pattern,str] <- been here as well
>                 ]
>             ]
>         ], 
>         str
>     ]
> ]

but then I thought if patterns was the right tool on the very defined and limited number of http methods, if it would improve anything but increase complexity.
On further inspection - e.g. (r'/test', [r'[a-z]+t$']) - I discarded it.

That said - I'd prefer

List[
    Union[
        Tuple[
            Union[Pattern,str],
            Optional[
                List[
                    Literal["get","put", "post", "delete","options","head","patch","trace"]
                ]
            ]
        ], 
        str
    ]
]

ggpwnkthx · 2023-09-27T14:56:54Z

Using Literal with the limited HTTP verbs is definitely the way to go.

I'm having trouble with pre-commit. The Github app doesn't seem to ever complete the commit. It's been sitting like this for almost a full day:

What am I doing wrong?

commonism · 2023-09-27T19:16:17Z

No idea about the github app, never used it.
pre-commit install a pre-commit hook to make sure everything is fine before accepting a commit - maybe here is a problem?
Try removing the pre-commit hook from .git/…

And I can imagine it is not happy about me re-basing and force pushing your branch.
Likely in case you did not reset&pull before making changes.
In case everything fails, just push the recent changes to a new branch on your repo and I'll pick it up.

commonism · 2023-09-28T12:50:51Z

tests/extra_test.py

        return ctx


 class MSGraphCulled(MSGraph, Cull):
-    pass
+    def paths(self, ctx: "Init.Context") -> "Init.Context":
+        return ctx


The Cull/Reduce paths() would set ctx.paths non and limit the initialization of schemas to schemas identified as required by tracking the schemas required for the operations.

Why avoid this?

I forget specifically what I was testing for there, but I think I was investigating something to do with schemas being ignored when they shouldn't be.

ADHD moment... It can be set back to pass for now, and I'll jump back into that investigation later.

remove - so the super() can take care.
tracking failing is a bug we'll address properly, if you find the schema/selectors required to reproduce.

Adjusted the Reduce class to handle a list of operations rather than a dict of path and methods. A string or pattern can be used in the list to match against operationIds, or a tuple with a string or pattern as the first element representing the path and a list of strings representing HTTP verbs as the second element will match against the path/methods. Also, in any case that a path is not culled, the non-operation keys are kept intact to maintain defaults and overrides. The MSGraph test was updated to accommodate the annotation change as well as to handle a few validation fixes. (cherry picked from commit 21b8dfb)

TODO: Investigate tracking failing (cherry picked from commit d00508b)

commonism · 2023-09-29T06:59:50Z

I adjusted the unit tests.

Now - as we work with a list, question …

Reduce(list) or Reduce(*list)

class Reduce(Document, Init):
    def __init__(
        self,
        operations: List[
            Union[Tuple[Union[re.Pattern, str], Optional[List[Union[re.Pattern, str]]]], Union[re.Pattern, str]]
        ],
    )

or

class Reduce(Document, Init):
    def __init__(
        self,
        *operations: List[
            Union[Tuple[Union[re.Pattern, str], Optional[List[Union[re.Pattern, str]]]], Union[re.Pattern, str]]
        ],
    )

As I do not exactly know if the typing is correct for the last one, example:

Reduce([("test",None), "test2"])

vs

Reduce(("test",None), "test2")

commonism force-pushed the master branch from fd0277e to 3eabbd1 Compare September 22, 2023 06:58

ggpwnkthx and others added 7 commits September 26, 2023 08:06

request - index operator interface

ea015fd

Small QOL update to the OperationIndex class that adds the __getitem__ method to enable access to operations via index operations (square brackets).

extra/cull - rebuild tags

c8c63c9

OpenAPI - disable print

d25eaf2

pre-commit formattings

84f19b5

tests - add test for request index operator

4e2fc1b

OpenAPI - types

fe2ee66

commonism force-pushed the master branch from 6b58d47 to 9c7b594 Compare September 26, 2023 06:08

extra - the operation methods can be None

f69e2df

commonism reviewed Sep 28, 2023

View reviewed changes

ggpwnkthx and others added 3 commits September 29, 2023 08:40

remove

8b4ad67

TODO: Investigate tracking failing (cherry picked from commit d00508b)

tests - extra changes

a13e86e

commonism force-pushed the master branch from d00508b to a13e86e Compare September 29, 2023 06:52

extra - use *args

14bee50

commonism merged commit 86f75e1 into commonism:master Oct 17, 2023
6 of 7 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

index operator interface #165

index operator interface #165

ggpwnkthx commented Sep 21, 2023

codecov bot commented Sep 21, 2023 •

edited

Loading

commonism commented Sep 22, 2023

ggpwnkthx commented Sep 25, 2023 •

edited

Loading

commonism commented Sep 26, 2023

commonism commented Sep 26, 2023

commonism commented Sep 26, 2023

ggpwnkthx commented Sep 26, 2023 •

edited

Loading

commonism commented Sep 26, 2023

ggpwnkthx commented Sep 27, 2023

commonism commented Sep 27, 2023

commonism Sep 28, 2023

ggpwnkthx Sep 28, 2023

commonism Sep 28, 2023

commonism commented Sep 29, 2023

index operator interface #165

index operator interface #165

Conversation

ggpwnkthx commented Sep 21, 2023

codecov bot commented Sep 21, 2023 • edited Loading

Codecov Report

commonism commented Sep 22, 2023

ggpwnkthx commented Sep 25, 2023 • edited Loading

commonism commented Sep 26, 2023

commonism commented Sep 26, 2023

commonism commented Sep 26, 2023

ggpwnkthx commented Sep 26, 2023 • edited Loading

commonism commented Sep 26, 2023

ggpwnkthx commented Sep 27, 2023

commonism commented Sep 27, 2023

commonism Sep 28, 2023

Choose a reason for hiding this comment

ggpwnkthx Sep 28, 2023

Choose a reason for hiding this comment

commonism Sep 28, 2023

Choose a reason for hiding this comment

commonism commented Sep 29, 2023

codecov bot commented Sep 21, 2023 •

edited

Loading

ggpwnkthx commented Sep 25, 2023 •

edited

Loading

ggpwnkthx commented Sep 26, 2023 •

edited

Loading