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

Federation Support - Description of custom directives breaks make_federated_schema #775

Closed
j-riebe opened this issue Jan 19, 2022 · 5 comments

Comments

@j-riebe
Copy link
Contributor

j-riebe commented Jan 19, 2022

I stumpled into a problem with the federation support while using custom directives.
It appears that make_federated_schema doesn't support custom directives if they have a description.

The culprit is following function:

def purge_schema_directives(joined_type_defs: str) -> str:
"""Remove custom schema directives from federation."""
joined_type_defs = _r_directive_definition.sub("", joined_type_defs)
joined_type_defs = _r_directive.sub(
lambda m: m.group(1) if m.group(2) in _allowed_directives else "",
joined_type_defs,
)
return joined_type_defs

It purges the directives from the schema but keeps the description in place.
This may lead to a violation of the GraphQL Syntax depending on which type definition come next.

The solution would be to extend the regex used to capture the directive definition so that it includes block-strings and single-line-strings.

_i_token_delimiter = r"(?:^|[\s\r\n]+|$)"
_i_token_name = "[_A-Za-z][_0-9A-Za-z]*"
_i_token_arguments = r"\([^)]*\)"
_i_token_location = "[_A-Za-z][_0-9A-Za-z]*"
_r_directive_definition = re.compile(
"("
f"{_i_token_delimiter}directive"
f"(?:{_i_token_delimiter})?@({_i_token_name})"
f"(?:(?:{_i_token_delimiter})?{_i_token_arguments})?"
f"{_i_token_delimiter}on"
f"{_i_token_delimiter}(?:[|]{_i_token_delimiter})?{_i_token_location}"
f"(?:{_i_token_delimiter}[|]{_i_token_delimiter}{_i_token_location})*"
")"
f"(?={_i_token_delimiter})",
)

@j-riebe
Copy link
Contributor Author

j-riebe commented Feb 25, 2022

Thx @rafalp

@j-riebe j-riebe closed this as completed Feb 25, 2022
@vikramsubramanian
Copy link

It feels like this Regex change would make parsing comments take forever (exponential?)

@j-riebe
Copy link
Contributor Author

j-riebe commented Apr 18, 2022

What do you mean by would and it feels?

Are you experiencing "forever"? 😄

@vikramsubramanian
Copy link

Hey, sorry for the vagueness.
Every """...""" comment block seems to exponentially grow the time it takes to parse a Graphql block. Simply removing all my comments compiled my code instantly whereas with ~10 comments, the script took >30 mins and ~30s with 1 comment block

@j-riebe
Copy link
Contributor Author

j-riebe commented Apr 19, 2022

I'd suggest to open a new issue, that targets the performance issue (as this issue is closed).

Some closing thoughts:
Keep in mind that the original problem is rooted in the use of purge_schema_directives which is reused in make_federated_schema

def purge_schema_directives(joined_type_defs: str) -> str:
"""Remove custom schema directives from federation."""
joined_type_defs = _r_directive_definition.sub("", joined_type_defs)
joined_type_defs = _r_directive.sub(
lambda m: m.group(1) if m.group(2) in _allowed_directives else "",
joined_type_defs,
)
return joined_type_defs

# Remove custom schema directives (to avoid apollo-gateway crashes).
# NOTE: This does NOT interfere with ariadne's directives support.
sdl = purge_schema_directives(type_defs)
type_token = "extend type" if has_query_type(sdl) else "type"
federation_service_type = federation_service_type_defs.format(type_token=type_token)

Maybe the true solution lies in removing purge_schema_directives entirely, as its only usage comes with a hacky comment:

(to avoid apollo-gateway crashes)

If one finds another way to avoid apollo-gateway crashes, we don't need that regex.

That being said, #736 also mentions this specific function. Maybe you can find some help there.

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

No branches or pull requests

2 participants