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

unable to configure federation directive namespace #201

Closed
thumbnail opened this issue Jul 5, 2022 · 7 comments
Closed

unable to configure federation directive namespace #201

thumbnail opened this issue Jul 5, 2022 · 7 comments

Comments

@thumbnail
Copy link

In federation version 2 the directives have namespaces (federation__ by default). Currently this prefix is non-configurable which may cause issues when other subgraphs use a different namespace or import them without a namespace.

In Apollo studio I got the following error:

The federation "@inaccessible" directive is imported with mismatched name between subgraphs: it is imported as "@inaccessible" in subgraphs "X" and "Y" but "@federation__inaccessible" in subgraphs "A" and "B"

In this case, A and B are ruby applications, and X and Y are not.

It can be partially fixed by configuring the graph as v1 and overriding Schema#federation_dsl to prepend an import for all directives. But it's not very elegant 😬

@geshwho
Copy link
Collaborator

geshwho commented Jul 5, 2022

Hey @thumbnail, thanks for reporting this! @daemonsy and I will take a look this week.

@geshwho
Copy link
Collaborator

geshwho commented Jul 6, 2022

Hey @thumbnail, we took a look at this today and it seems like there are a couple of enhancements that would help us get closer to spec compliant with federation version 2.

  • We need to provide an option that allows the user to override the default namespace (similar to the example in the docs which uses fed2 as the namespace)
  • We need to provide an option for users to specify the import argument on the link directive such that they will not be prefixed. From the docs:

The import argument enables you to import external definitions into your namespace, so they are not prefixed.

I believe the latter one would help resolve the issue that you are seeing. Do you have any interest in working on a solution for either of these enhancements?

@geshwho
Copy link
Collaborator

geshwho commented Jul 12, 2022

Hey @lennyburdette, I was just rereading your comment here and realized that it directly conflicts with what I just suggested above. Namely, per your comment, code-first libraries will not be providing an API to rename imports (second bullet point). However, I wanted to ask: should we consider an API to allow overriding the default namespace (first bullet point)?

@lennyburdette
Copy link
Contributor

Hmm this is odd. It appears to me that this error occurs with @inaccessible but not @key or @shareable so I think this is a bug related to backward-compatibility with @tag and @inaccessible. I'll reach out to the team tomorrow and get back to you.

https://github.com/apollographql/federation/blob/main/composition-js/src/merging/merge.ts#L198-L201
https://github.com/apollographql/federation/blob/main/composition-js/src/merging/merge.ts#L285-L287
https://github.com/apollographql/federation/blob/main/composition-js/src/merging/merge.ts#L292-L322

@geshwho
Copy link
Collaborator

geshwho commented Jul 12, 2022

Ah, good point! This should be backwards compatible. We'll wait until we hear more from the team before we do anything.

@lennyburdette
Copy link
Contributor

I had a good conversation with my team about this and learned a bunch. I still consider this a bug, but it may not be a high priority to fix. And we'll eventually need an API for configuring future @linked directives, so it's probably worth addressing now.

Here's my summary of the conversation: apollographql/federation#1976 (comment)

@grxy
Copy link
Collaborator

grxy commented Mar 8, 2023

I'm going to close this since this was solved in 03fdfea

@grxy grxy closed this as completed Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants