Federation v2 support added #4

arun-sureshkumar · 2022-10-04T01:06:26Z

Add Support to Federation v2

apollo-federation-subgraph-compatibility - Passed

arun-sureshkumar · 2022-10-06T00:33:37Z

@patrick91 - apollo-federation-subgraph-compatibility Passed

arun-sureshkumar · 2022-10-06T03:47:33Z

@erikwrede - Can you please review the PR?

erikwrede · 2022-10-06T15:25:18Z

@arun-strollby great job! I'll have a look in the weekend, but the final word should be on @tcleonard or @patrick91, since they are more experienced with federation.

coveralls · 2022-10-06T15:29:27Z

Pull Request Test Coverage Report for Build 3196356399

152 of 196 (77.55%) changed or added relevant lines in 11 files are covered.
3 unchanged lines in 1 file lost coverage.
Overall coverage decreased (-7.8%) to 83.25%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
graphene_federation/override.py	10	14	71.43%
graphene_federation/tag.py	10	14	71.43%
graphene_federation/service.py	43	54	79.63%
graphene_federation/inaccessible.py	19	31	61.29%
graphene_federation/shareable.py	19	32	59.38%

Files with Coverage Reduction	New Missed Lines	%
graphene_federation/service.py	3	88.33%

Totals
Change from base Build 3015879032:	-7.8%
Covered Lines:	333
Relevant Lines:	400

💛 - Coveralls

coveralls · 2022-10-06T15:29:28Z

Pull Request Test Coverage Report for Build 3196356399

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

155 of 188 (82.45%) changed or added relevant lines in 13 files are covered.
28 unchanged lines in 4 files lost coverage.
Overall coverage decreased (-7.8%) to 83.25%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
graphene_federation/entity.py	12	13	92.31%
graphene_federation/override.py	6	8	75.0%
graphene_federation/tag.py	6	8	75.0%
graphene_federation/service.py	60	65	92.31%
graphene_federation/inaccessible.py	11	21	52.38%
graphene_federation/shareable.py	19	32	59.38%

Files with Coverage Reduction	New Missed Lines	%
graphene_federation/provides.py	1	96.67%
graphene_federation/types.py	3	75.0%
graphene_federation/service.py	9	88.33%
graphene_federation/entity.py	15	70.37%

Totals
Change from base Build 3015879032:	-7.8%
Covered Lines:	333
Relevant Lines:	400

💛 - Coveralls

erikwrede · 2023-01-30T09:07:30Z

gentle push, let's get this over the finish line 🙂 @patrick91 @arun-sureshkumar @adarshdigievo

patrick91 · 2023-02-20T20:15:09Z

@arun-sureshkumar @arunsureshkumar @adarshdigievo are you planning to update this PR or shall I take over? 😊

arunsureshkumar · 2023-02-21T04:06:31Z

Patrick, I will close this by end of this week. Thanks, Arun

…

On Tue, 21 Feb 2023 at 01:45, Patrick Arminio ***@***.***> wrote: @arun-sureshkumar <https://github.com/arun-sureshkumar> @arunsureshkumar <https://github.com/arunsureshkumar> @adarshdigievo <https://github.com/adarshdigievo> are you planning to update this PR or shall I take over? 😊 — Reply to this email directly, view it on GitHub <#4 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEIAZMGF4ITLPMLB7676PC3WYPGFRANCNFSM6AAAAAAQ4DCYVE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Meemaw · 2023-03-07T11:35:10Z

@arunsureshkumar any update on getting this in?

patrick91 · 2023-03-14T13:11:42Z

@arun-sureshkumar let me know if you want us to update the PR 😊 happy to take over it :D

…ort-federation-v2 # Conflicts: # README.md # examples/inaccessible.py # examples/override.py # examples/shareable.py # examples/tag.py # graphene_federation/entity.py # graphene_federation/extend.py # graphene_federation/external.py # graphene_federation/inaccessible.py # graphene_federation/override.py # graphene_federation/provides.py # graphene_federation/requires.py # graphene_federation/service.py # graphene_federation/shareable.py # graphene_federation/tag.py # graphene_federation/tests/test_key.py

# Conflicts: # graphene_federation/extend.py # graphene_federation/service.py

arun-sureshkumar · 2023-03-14T14:12:39Z

@arun-sureshkumar let me know if you want us to update the PR 😊 happy to take over it :D

@patrick91 Please check now, let me know your comments!

arun-sureshkumar · 2023-03-14T19:42:54Z

Currently playing around with the PR for my final review and it's working great so far!

I have some minor remarks. We should discuss the behavior of the @shareable directive on interfaces:
@shareable
class ReviewInterface(Interface):
    interfaced_body = graphene.String()

class Review(ObjectType):
    class Meta:
        interfaces = (ReviewInterface,)

    id = graphene.ID()
    body = graphene.String()
    interfaced_body = graphene.String()

    def resolve_interfaced_body(self, info):
        return "".join([self.body, "interfaced"])
Currently, this ignores the directive on the interfaces (i.e. it is not included in the SDL). According to the Federation Docs, starting from federation v2.2 this should not be allowed instead of ignored:

The @Shareable directive is about indicating when an object field can be resolved by multiple subgraphs. As interface fields are not directly resolved (their implementation is), @Shareable is not meaningful on an interface field and is not allowed (at least since federation 2.2; earlier versions of federation 2 mistakenly ignored @Shareable on interface fields).

Since we are just introducing Federation v2 here, maybe we should adopt this behavior from the start to avoid making breaking changes later on. What do you guys think?

This will not be allowed instead of ignored, updating the code.

Fix: Lint Error Passed: Test Cases

arun-sureshkumar · 2023-03-14T19:49:31Z

Correcting my above post, it seems like Interfaces in general do not work with the directives yet. For example: inaccessible on graphene.Interface doesn't seem to work either, using strawberry federation as a working python reference:
@strawberry.interface(directives=[Inaccessible()])
class ReviewInterface:
    interfaced_body: str

@strawberry.type
class Review(ReviewInterface):
    id: int
    body: str
Resulting SDL:
type Review implements ReviewInterface {
  interfacedBody: String!
  id: Int!
  body: String!
}

interface ReviewInterface @inaccessible {
  interfacedBody: String!
}
Versus the SDL from my example above using @inaccessible instead of @shareable:
type Review implements ReviewInterface {
  interfacedBody: String
  id: ID
  body: String
}

interface ReviewInterface {
  interfacedBody: String
}
As a consequence, ReviewInterface is included in the stitched schema.

Seems like the Apollo Subgraph Compatibility test doesn't incorporate this case, which is probably why it was missed. Maybe it makes sense to add some additional tests specifically for Interfaces here. On the other hand, I don't want to hold the release of this back if it's too much work to add. What do you guys think? @patrick91 @arun-sureshkumar

This issue is fixed.

erikwrede · 2023-03-14T20:34:34Z

Awesome, thanks for the changes! Can we add some tests for the added behavior? Apart from that LGTM!

arun-sureshkumar · 2023-03-14T20:36:51Z

Awesome, thanks for the changes! Can we add some tests for the added behavior? Apart from that LGTM!

Sure thing!

arun-sureshkumar · 2023-03-14T21:12:59Z

Awesome, thanks for the changes! Can we add some tests for the added behavior? Apart from that LGTM!

Sure thing!

Please check and let me know if anything to be added.

patrick91 · 2023-03-20T21:51:39Z

@arun-sureshkumar @erikwrede I'll do a review in the couple of days 😊

patrick91 · 2023-03-22T21:48:02Z

Just tested this locally and it seems to work well :D @erikwrede what are the next steps for getting this released?

erikwrede · 2023-03-24T10:40:12Z

Thanks everyone for the amazing work on this! 🎉

arun-sureshkumar added 6 commits October 3, 2022 18:01

Federation v2 support added

dba69d0

Minor fix in link

308be36

Minor fix in @provides

c61f145

Minor fix in @keys

5bb34d7

Revert patch in @keys

c469282

Remove Print

97f6fdc

patrick91 assigned patrick91 and unassigned patrick91 Oct 4, 2022

patrick91 self-requested a review October 4, 2022 04:57

arun-sureshkumar force-pushed the support-federation-v2 branch from 13e91af to 97f6fdc Compare October 4, 2022 05:37

arun-sureshkumar added 11 commits October 3, 2022 22:44

add override @OverRide to @link

1489d96

fix @link

8e5d39f

fix @link

d8d8240

fix @link

963e0a2

Test case passed

f1ea7b8

Add @key (multi) support

70b9476

Fix Test Cases

c7aa475

Fix Test Cases

783bfb7

Fix Lint Error

2438b3d

Update README.md

f0c397b

Fix Lint Error

c4616fb

erikwrede requested a review from tcleonard October 6, 2022 07:31

Multi fields support @extend

e625658

arun-sureshkumar force-pushed the support-federation-v2 branch 2 times, most recently from b5a7e67 to 10bcb9a Compare October 6, 2022 10:15

Merge branch 'graphql-python:main' into support-federation-v2

6644a6d

erikwrede mentioned this pull request Feb 23, 2023

@include and @skip directives lead to irrelevant errors graphql-python/graphene#1496

Closed

arunsureshkumar added 3 commits March 14, 2023 18:48

Support @inaccessible on graphene Interface

3ebf1f9

Merge remote-tracking branch 'original/main' into support-federation-v2

72e0d14

# Conflicts: # graphene_federation/extend.py # graphene_federation/service.py

arunsureshkumar added 3 commits March 14, 2023 19:45

Fix: Lint Error

f32186d

Fix: Gateway Dockerfile build error

9c1e6ed

Add Support: @inaccessible & @Shareable to graphene.Union

1fb0ffe

Update: Dependencies, Dockerfile

a479fc5

Fix: Lint Error Passed: Test Cases

arun-sureshkumar requested review from patrick91 and removed request for tcleonard March 14, 2023 19:50

Add: Test case for @Shareable & @inaccessible

01b9f9a

arun-sureshkumar requested review from erikwrede and removed request for patrick91 March 14, 2023 21:25

Fix: Lint Error

e6d7fe0

patrick91 merged commit 8b5e1e7 into graphql-python:main Mar 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Federation v2 support added #4

Federation v2 support added #4

arun-sureshkumar commented Oct 4, 2022 •

edited

Loading

arun-sureshkumar commented Oct 6, 2022

arun-sureshkumar commented Oct 6, 2022

erikwrede commented Oct 6, 2022

coveralls commented Oct 6, 2022

coveralls commented Oct 6, 2022 •

edited

Loading

erikwrede commented Jan 30, 2023

patrick91 commented Feb 20, 2023

arunsureshkumar commented Feb 21, 2023 via email

Meemaw commented Mar 7, 2023 •

edited

Loading

patrick91 commented Mar 14, 2023

arun-sureshkumar commented Mar 14, 2023 •

edited

Loading

arun-sureshkumar commented Mar 14, 2023

arun-sureshkumar commented Mar 14, 2023

erikwrede commented Mar 14, 2023

arun-sureshkumar commented Mar 14, 2023

arun-sureshkumar commented Mar 14, 2023

patrick91 commented Mar 20, 2023

patrick91 commented Mar 22, 2023

erikwrede commented Mar 24, 2023

Federation v2 support added #4

Federation v2 support added #4

Conversation

arun-sureshkumar commented Oct 4, 2022 • edited Loading

arun-sureshkumar commented Oct 6, 2022

arun-sureshkumar commented Oct 6, 2022

erikwrede commented Oct 6, 2022

coveralls commented Oct 6, 2022

Pull Request Test Coverage Report for Build 3196356399

💛 - Coveralls

coveralls commented Oct 6, 2022 • edited Loading

Pull Request Test Coverage Report for Build 3196356399

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

erikwrede commented Jan 30, 2023

patrick91 commented Feb 20, 2023

arunsureshkumar commented Feb 21, 2023 via email

Meemaw commented Mar 7, 2023 • edited Loading

patrick91 commented Mar 14, 2023

arun-sureshkumar commented Mar 14, 2023 • edited Loading

arun-sureshkumar commented Mar 14, 2023

arun-sureshkumar commented Mar 14, 2023

erikwrede commented Mar 14, 2023

arun-sureshkumar commented Mar 14, 2023

arun-sureshkumar commented Mar 14, 2023

patrick91 commented Mar 20, 2023

patrick91 commented Mar 22, 2023

erikwrede commented Mar 24, 2023

arun-sureshkumar commented Oct 4, 2022 •

edited

Loading

coveralls commented Oct 6, 2022 •

edited

Loading

Meemaw commented Mar 7, 2023 •

edited

Loading

arun-sureshkumar commented Mar 14, 2023 •

edited

Loading