Allow codegen to process multiple query files in a single command #2911

mgilson · 2023-06-30T17:20:57Z

Description

#2828 hopefully does a good job communicating the use-case for this feature. Currently codegen can be very slow if the schema import is an expensive operation. This allows a user to codegen multiple files with a single command (and a single schema import). There are a few minor (backward compatible) extensions to the exposed QueryCodegenPlugin and ConsolePlugin interfaces. The RELEASE.md should describe these changes and the class behaviors in their entirety.

Types of Changes

Issues Fixed or Closed by This PR

Allow codegen to process multiple queries #2828

Checklist

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
I have tested the changes and verified that they work and don't break anything (as well as I can manage).

codecov · 2023-06-30T17:23:26Z

Codecov Report

Merging #2911 (dfe9d77) into main (6d86d1c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2911   +/-   ##
=======================================
  Coverage   96.55%   96.55%           
=======================================
  Files         468      468           
  Lines       29201    29265   +64     
  Branches     3592     3603   +11     
=======================================
+ Hits        28194    28257   +63     
  Misses        827      827           
- Partials      180      181    +1

botberry · 2023-06-30T17:23:39Z

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

strawberry codegen can now operate on multiple input query files.
The previous behavior of naming the file types.js and types.py
for the builtin typescript and python plugins respectively is
preserved, but only if a single query file is passed. When more
than one query file is passed, the code generator will now use
the stem of the query file's name to construct the name of the
output files. e.g. my_query.graphql -> my_query.js or
my_query.py. Creators of custom plugins are responsible
for controlling the name of the output file themselves. To
accomodate this, if the __init__ method of a QueryCodegenPlugin
has a parameter named query or query_file, the pathlib.Path
to the query file will be passed to the plugin's __init__
method.

Finally, the ConsolePlugin has also recieved two new lifecycle
methods. Unlike other QueryCodegenPlugin, the same instance of
the ConsolePlugin is used for each query file processed. This
allows it to keep state around how many total files were processed.
The ConsolePlugin recieved two new lifecycle hooks: before_any_start
and after_all_finished that get called at the appropriate times.

Here's the preview release card for twitter:

Here's the tweet text:

🆕 Release (next) is out! Thanks to Matt Gilson for the PR 👏

Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)

botberry · 2023-06-30T17:23:58Z

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

strawberry codegen can now operate on multiple input query files.
The previous behavior of naming the file types.js and types.py
for the builtin typescript and python plugins respectively is
preserved, but only if a single query file is passed. When more
than one query file is passed, the code generator will now use
the stem of the query file's name to construct the name of the
output files. e.g. my_query.graphql -> my_query.js or
my_query.py. Creators of custom plugins are responsible
for controlling the name of the output file themselves. To
accomodate this, if the __init__ method of a QueryCodegenPlugin
has a parameter named query or query_file, the pathlib.Path
to the query file will be passed to the plugin's __init__
method.

Finally, the ConsolePlugin has also recieved two new lifecycle
methods. Unlike other QueryCodegenPlugin, the same instance of
the ConsolePlugin is used for each query file processed. This
allows it to keep state around how many total files were processed.
The ConsolePlugin recieved two new lifecycle hooks: before_any_start
and after_all_finished that get called at the appropriate times.

Here's the preview release card for twitter:

Here's the tweet text:

🆕 Release (next) is out! Thanks to Matt Gilson for the PR 👏

Get it here 👉 https://github.com/strawberry-graphql/strawberry/releases/tag/(next)

mgilson · 2023-07-03T01:36:04Z

tests/cli/test_codegen.py

+        assert " GetUserResult" in path.read_text()
+
+
+def test_codegen_pass_no_query(cli_runner: CliRunner, tmp_path: Path):


I don't have an opinion on the proper behavior here. I went with the behavior recommended by click for variadic arguments

... we believe scripts should gracefully degrade into becoming noops if a variadic argument is empty. The reason for this is that very often, scripts are invoked with wildcard inputs from the command line and they should not error out if the wildcard is empty.

mgilson · 2023-07-03T01:44:10Z

strawberry/cli/commands/codegen.py

+        warnings.warn(
+            "The ConsolePlugin should inherit from ``{__name__}.ConsolePlugin``.",
+            DeprecationWarning,
+            stacklevel=1,


ruff wants the explicit stacklevel 🤷‍♂️ .

yeah, that's fine :D

codspeed-hq · 2023-07-18T13:20:28Z

CodSpeed Performance Report

Merging #2911 will not alter performance

_{Comparing mgilson:multiple-codegen-revised (dfe9d77) with main (6d86d1c)}

Summary

✅ 12 untouched benchmarks

patrick91 · 2023-08-07T22:20:46Z

@mgilson I'll try to review this during this week 😊 thanks for your patience!

patrick91

left some comments, I think this is almost ready to merge :)

patrick91 · 2023-08-24T12:29:34Z

strawberry/cli/commands/codegen.py

+        sig = inspect.signature(ptype)
+        kwargs = {}
+        if "query" in sig.parameters:
+            kwargs["query"] = query
+        if "query_file" in sig.parameters:
+            kwargs["query_file"] = query
+
+        plugin = ptype(**kwargs)
+        if not hasattr(plugin, "query"):
+            plugin.query = query


is this to keep backwards compatibility?

yeah, I did a lot of shenanigans to try to preserve compatibility with any existing plugins that might be in the wild.

If we don't care about that, then we can probably simplify this a good bit.

I think it might be fine to break compatibility for this, we can just add a note in our breaking changes files. We could do a codemod too but it is probably not worth doing 😊

patrick91 · 2023-08-24T12:30:33Z

strawberry/cli/commands/codegen.py

+        warnings.warn(
+            "The ConsolePlugin should inherit from ``{__name__}.ConsolePlugin``.",
+            DeprecationWarning,
+            stacklevel=1,


yeah, that's fine :D

strawberry/cli/commands/codegen.py

patrick91 · 2023-08-24T12:32:52Z

strawberry/cli/commands/codegen.py

+    plugins = _load_plugins(selected_plugins, query[0])
+    console_plugin = console_plugin_type(query[0], output_dir, plugins)


I'm not sure I like having to pass the first query here 🤔

Yeah, I dislike this as well. I'd rather have the console_plugin_type accept a collection of queries, but anyone who has made a custom console plugin type would potentially be broken in that case :(.

strawberry/cli/commands/codegen.py

mgilson · 2023-08-25T17:36:05Z

strawberry/cli/commands/codegen.py


 if TYPE_CHECKING:
-    from strawberry.codegen import CodegenResult
+    from pathlib import Path


AHH!!!

What tool is actually moving this import?

In this case, the tool is thinking that Path should be in an if TYPE_CHECKING because it's only used in annotations, however, typer is introspecting the types so the import statement needs to be present at runtime.

I'm not sure how to prevent our tool from moving this (other than putting in some sort of useless expression that actually uses Path somewhere).

that's ruff, I think that rule is becoming quite annoying, # noqa: TCH001 should fix it

patrick91 · 2023-09-08T21:50:55Z

@mgilson really sorry for the delay on this :)

I think we can merge as it is now, I've changed the plugins to use the filename of the query to generate the files 😊

I think maybe in future we can customise it more to make so that the generated files are generated near the source ones and not passing the output directory at all 😊

let me know if the changes look good to you and I'll merge

mgilson

Looks great to me!

…rawberry-graphql#2911) * Add ability for `codegen` to handle multiple input query files. * Add a `RELEASE.md` file. * Update how we name generated files --------- Co-authored-by: Matt Gilson <mgilson@lat.ai> Co-authored-by: Patrick Arminio <patrick.arminio@gmail.com>

botberry added bot:has-release-file bot:release-type-minor labels Jun 30, 2023

mgilson force-pushed the multiple-codegen-revised branch 5 times, most recently from f668377 to aa6a6fc Compare July 3, 2023 01:30

mgilson commented Jul 3, 2023

View reviewed changes

mgilson force-pushed the multiple-codegen-revised branch from 0fbf48d to 55d6e2d Compare July 3, 2023 01:43

mgilson commented Jul 3, 2023

View reviewed changes

mgilson force-pushed the multiple-codegen-revised branch from 9a4cd6b to 6811a81 Compare July 3, 2023 14:29

patrick91 self-requested a review July 4, 2023 19:18

mgilson force-pushed the multiple-codegen-revised branch from 6811a81 to d45dc52 Compare July 18, 2023 13:17

patrick91 reviewed Aug 24, 2023

View reviewed changes

mgilson force-pushed the multiple-codegen-revised branch 5 times, most recently from b8e4e43 to f3de817 Compare August 25, 2023 17:18

mgilson commented Aug 25, 2023

View reviewed changes

mgilson force-pushed the multiple-codegen-revised branch 5 times, most recently from c6f6d8c to 7756571 Compare August 25, 2023 21:30

Matt Gilson added 2 commits August 25, 2023 20:21

Add ability for codegen to handle multiple input query files.

26dda14

Add a RELEASE.md file.

73411da

mgilson force-pushed the multiple-codegen-revised branch from a89aa95 to 73411da Compare August 26, 2023 00:21

Update how we name generated files

dfe9d77

mgilson commented Sep 9, 2023

View reviewed changes

patrick91 merged commit 3c30ef1 into strawberry-graphql:main Sep 13, 2023
55 of 57 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow codegen to process multiple query files in a single command #2911

Allow codegen to process multiple query files in a single command #2911

mgilson commented Jun 30, 2023

codecov bot commented Jun 30, 2023 •

edited

Loading

botberry commented Jun 30, 2023 •

edited

Loading

botberry commented Jun 30, 2023

mgilson Jul 3, 2023

mgilson Jul 3, 2023

patrick91 Aug 24, 2023

codspeed-hq bot commented Jul 18, 2023 •

edited

Loading

patrick91 commented Aug 7, 2023

patrick91 left a comment

patrick91 Aug 24, 2023

mgilson Aug 24, 2023

patrick91 Aug 24, 2023

patrick91 Aug 24, 2023

patrick91 Aug 24, 2023

mgilson Aug 24, 2023

mgilson Aug 25, 2023

patrick91 Aug 25, 2023

patrick91 commented Sep 8, 2023

mgilson left a comment

		assert " GetUserResult" in path.read_text()


		def test_codegen_pass_no_query(cli_runner: CliRunner, tmp_path: Path):

		plugins = _load_plugins(selected_plugins, query[0])
		console_plugin = console_plugin_type(query[0], output_dir, plugins)

Allow codegen to process multiple query files in a single command #2911

Allow codegen to process multiple query files in a single command #2911

Conversation

mgilson commented Jun 30, 2023

Description

Types of Changes

Issues Fixed or Closed by This PR

Checklist

codecov bot commented Jun 30, 2023 • edited Loading

Codecov Report

botberry commented Jun 30, 2023 • edited Loading

botberry commented Jun 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codspeed-hq bot commented Jul 18, 2023 • edited Loading

Merging #2911 will not alter performance

Summary

patrick91 commented Aug 7, 2023

patrick91 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrick91 commented Sep 8, 2023

mgilson left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 30, 2023 •

edited

Loading

botberry commented Jun 30, 2023 •

edited

Loading

codspeed-hq bot commented Jul 18, 2023 •

edited

Loading