Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

impl filters and tests #1857

Merged
merged 12 commits into from
Nov 22, 2024
Merged

impl filters and tests #1857

merged 12 commits into from
Nov 22, 2024

Conversation

shivamka1
Copy link
Collaborator

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change? If yes is this documented?

How was this patch tested?

Issues

If this resolves any issues, please link to them here, the format is a KEYWORD followed by @_
KEYWORDS available are close, closes, closed, fix, fixes, fixed, resolve, resolves, resolved.
Please delete this text before creating your PR

Are there any further changes required?

@shivamka1
Copy link
Collaborator Author

Initial ideas on what shapes these apis will be taking going forward alongside developments in lib.

query w {
  graph(path: "mygraph1", nodeFilter: {}, edgeFilter: {}) {
    # nodeFilter() {}
    # edgeFitler() {}
    
    nodes(nodeFilter: {
      or: [
        {and: [{prop: {field: "a", ge: 1}}, {type: ["a","b","c"]}]},
        {prop: {field: "b", any: [1,2,3]}}
      ]
    }) {
      list {
        name
      }
  	}
	}
}

query w2 {
  graph(path: "mygraph1") {
    # nodePropertyFilter() {}
    # edgePropertyFitler() {}
    
    n1: nodes(nodeFilter: {
      and: [{propFilter: {field: "a", ge: 1}}, {typeFilter: ["a","b","c"]}]
    }) {
      list {
        name
      }
  	}
    
    n2: nodes(nodeFilter: {
      propFilter: {field: "a", ge: 1}
    }) {
      list {
        name
      }
  	}
	}
}

@shivamka1 shivamka1 marked this pull request as ready for review November 15, 2024 09:55
Copy link
Collaborator

@miratepuffin miratepuffin left a comment

Choose a reason for hiding this comment

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

First half of review - looking through the graph tests now

python/tests/graphql/test_nodes_property_filter.py Outdated Show resolved Hide resolved
python/tests/graphql/test_nodes_property_filter.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@miratepuffin miratepuffin left a comment

Choose a reason for hiding this comment

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

For edges I have the same requests as nodes. Minus you cannot apply an edge filter multiple times and you have already checked that the equality works for lists

@miratepuffin miratepuffin merged commit 7ab3c76 into master Nov 22, 2024
19 checks passed
@miratepuffin miratepuffin deleted the features/filter branch November 22, 2024 14:59
shivamka1 added a commit that referenced this pull request Nov 22, 2024
miratepuffin added a commit that referenced this pull request Nov 22, 2024
* impl import node as

* impl import nodes as

* impl import node/nodes as tests

* add import edge/edges as tests

* fix error msgs

* impl import as for py

* fix doc comments

* add review changes and fix tests

* ref

* ref, add tests

* fix storage

* fix test from #1857

* ren force to merge

* fix docs

---------

Co-authored-by: Shivam Kapoor <[email protected]>
Co-authored-by: Ben Steer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants