-
Notifications
You must be signed in to change notification settings - Fork 61
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
Generate code as part of "composer install" #388
Comments
From https://getcomposer.org/doc/articles/scripts.md
https://getcomposer.org/doc/articles/plugins.md
We don't require our current plugins ( |
Update on this: There's a composer plugin now. The problem is that executing through sake and even just booting the kernel requires a database connection, which means we can't run this a build environment (e.g. CI) that's separate from the runtime environment - see WIP pull request. |
Since Versioned->versions() results in an ArrayList, it triggers database queries. The database isn't always available when the schema is built (e.g. on deployment and CI environments). Context: silverstripe/silverstripe-graphql#388
This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388
This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388
This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388
This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388
This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388
I think the 'Inclusion Option 1' makes the most sense, it seems to be the closest to the module that actually works with it. Installer or recipe-plugin are too far from the problem from my perspective. You've already mentioned the edge case where the generation will need to be compatible with codebases where the generated code was already committed to the repo. I'd also check how it behaves on fresh installs with pre-existing project code defining custom types. |
Had a long chat with Aaron, and we've tried to isolate the need for a database-less execution from GraphQL code gen a bit, which required choosing between a few different options for core behaviour. OverviewAt its core, Silverstripe is a web application framework, and since the ORM is bundled into the "core" the assumption of a valid database connection is pretty strong. The database connection is established lazy (when There are uncommon but still valid scenarios where a database connection is not available:
There is a WIP pull request for adding a NullDatabase to framework, a fake that throws exceptions if the database is used. In the first scenario (GraphQL code gen) it can point at docs to describe why querying is a bad idea in this context. It also relies on a Databaseless OptionsDatabaseless Option 1: Make CoreKernel bootstrap methods composableThis would be the most pluggable (for custom class CoreKernel
{
protected $bootstrappers = [
SilverStripe\Core\Bootstrapper\PHP::class,
SilverStripe\Core\Bootstrapper\Manifest::class,
SilverStripe\Core\Bootstrapper\DatabaseConfig::class,
// ...
];
public function setBootstrappers(array $bs)
{
$this->bootstrappers = $bs;
}
} Databaseless Option 2: Add behaviour modifiers to CoreKernelBig API surface increase, but retains our flexibility class CoreKernel
{
protected $bootDatabase = true;
public function boot()
{
$this->bootPHP();
if ($this->bootDatabase) {
$this->bootDatabase()
}
}
} Databaseless Option 3: Subclass CoreKernelSpecial purpose kernel, which worked when we just used it in class DatabaselessKernel extends CoreKernel
{
public function boot()
{
$this->bootPHP();
// leave out $this->bootDatabase()
// ...
}
} Databaseless Option 4: Lazy database validationMove This will force us to also remove the non-functional GraphQL Code Gen BehaviourWith any of the options proposed the framework can boot without requiring a database, ConclusionDatabaseless Option 4, since it avoids API changes as well as changes to highlevel usage (such as |
Since Versioned->versions() results in an ArrayList, it triggers database queries. The database isn't always available when the schema is built (e.g. on deployment and CI environments). Context: silverstripe/silverstripe-graphql#388
This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388
This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388
This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388
This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388
This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388
* NEW DatabaselessKernel to support operation without DB This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388 * New --no-database option for sake * Refactor to abstract class * Apply feedback peer review Co-authored-by: Aaron Carlino <[email protected]> Co-authored-by: Maxime Rainville <[email protected]>
* NEW DatabaselessKernel to support operation without DB This is required for GraphQL code generation in CI (without a working runtime database/webserver environment). Context: silverstripe/silverstripe-graphql#388 * New --no-database option for sake * Refactor to abstract class * Apply feedback peer review Co-authored-by: Aaron Carlino <[email protected]> Co-authored-by: Maxime Rainville <[email protected]>
This makes it consistent with the
vendor-expose
approach, and ensures that both new and upgrading devs have a good experience (especially when they don't read upgrading guides).As a reminder, there's the following ways of generating GraphQL code. Only the
sake dev/graphql/build
on dev envs and committing the result to the codebase. Multi server safe.dev/build
(on dev or production runtime envs). Not multi server safe.sake dev/graphql/build
on production runtime envs. Not multi server safe./graphql
handlers (withSilverStripe\GraphQL\Controller::$autobuildSchema
defaulting totrue
). Not multi server safe.We propose adding a new default method:
composer install
andcomposer update
. That only works for GraphQL configuration added/updated through modules, you'd still needsake dev/graphql/build
to develop your own APIs. But the advantage here is that these commands already run as part of deployment processes, and likely would transparently include the new.graphql/
folder.Note: Our own platforms run
composer install --no-scripts && composer vendor-expose
for security reasons, so we'll still need to add a new command to those.Inclusion Options
There are various options on how to introduce this new logic
Inclusion Option 1: Require
silverstripe/graphql-composer-plugin
insilverstripe/graphql
. This means everyone will run GraphQL code generation oncomposer install
andcomposer update
by default when upgrading to CMS 4.9, unless they opt out viaSS_GRAPHQL_COMPOSER_BUILD_SCHEMAS=''
. The opt-out would only be required if the generated GraphQL code is checked in to the codebase (ganky), or if there are alternative ways to generate it during deployment or at runtime in production.Inclusion Option 2: Require
silverstripe/graphql-composer-plugin
insilverstripe/installer
. This means only new installs will get it by default, and we'd strongly recommend opt-in to any sites upgrading to CMS 4.9. A bit more "room for error" in this new implementation, but it'll lead to lots of confusion when the "runtime fallbacks" described above kick in and behave inconsistently between servers.Inclusion Option 3: Add this logic to the
silverstripe/recipe-plugin
instead of a new composer plugin, which is already required by framework. In this case, we need to make the code generation a no-op in case graphql isn't installed.Tasks
sake dev/graphql/build
SilverStripe\GraphQL\Dev\DevelopmentAdmin
(add more specific friendly error to instance)Pull Requests
Testing this feature
Shortcut to set up a project for this (mostly for my own reference):
Now you can simulate a
composer install
run:There's more validation about operating this change on Silverstripe's own platforms in an internal ticket
The text was updated successfully, but these errors were encountered: