Skip to content

Make graphql-c_parser optional with warning when unavailable #1003

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.

Sign up for GitHub

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

Draft
myronmarston wants to merge 1 commit into
base: myron/remove-fork
Choose a base branch
from
Draft

Make graphql-c_parser optional with warning when unavailable #1003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions CODEBASE_OVERVIEW.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ graph LR;
elasticgraph-graphql["eg-graphql"];
base64["base64"];
graphql["graphql"];
graphql-c_parser["graphql-c_parser"];
hashdiff["hashdiff"];
rack["rack"];
logger["logger"];
Expand All @@ -52,7 +51,6 @@ graph LR;
elasticgraph-graphql --> elasticgraph-datastore_core;
elasticgraph-graphql --> elasticgraph-schema_artifacts;
elasticgraph-graphql --> graphql;
elasticgraph-graphql --> graphql-c_parser;
elasticgraph-indexer --> elasticgraph-datastore_core;
elasticgraph-indexer --> elasticgraph-schema_artifacts;
elasticgraph-indexer --> elasticgraph-support;
Expand All @@ -73,15 +71,13 @@ graph LR;
class elasticgraph-graphql targetGemStyle;
class base64 externalGemCatStyle;
class graphql externalGemCatStyle;
class graphql-c_parser externalGemCatStyle;
class hashdiff externalGemCatStyle;
class rack externalGemCatStyle;
class logger externalGemCatStyle;
class json_schemer externalGemCatStyle;
click rake href "https://rubygems.org/gems/rake" "Open on RubyGems.org" _blank;
click base64 href "https://rubygems.org/gems/base64" "Open on RubyGems.org" _blank;
click graphql href "https://rubygems.org/gems/graphql" "Open on RubyGems.org" _blank;
click graphql-c_parser href "https://rubygems.org/gems/graphql-c_parser" "Open on RubyGems.org" _blank;
click hashdiff href "https://rubygems.org/gems/hashdiff" "Open on RubyGems.org" _blank;
click rack href "https://rubygems.org/gems/rack" "Open on RubyGems.org" _blank;
click logger href "https://rubygems.org/gems/logger" "Open on RubyGems.org" _blank;
Expand Down Expand Up @@ -117,7 +113,6 @@ graph LR;
webrick["webrick"];
elasticgraph-schema_artifacts["eg-schema_artifacts"];
graphql["graphql"];
graphql-c_parser["graphql-c_parser"];
elasticgraph --> elasticgraph-support;
elasticgraph --> thor;
elasticgraph-local --> elasticgraph-admin;
Expand All @@ -133,7 +128,6 @@ graph LR;
elasticgraph-schema_definition --> elasticgraph-schema_artifacts;
elasticgraph-schema_definition --> elasticgraph-support;
elasticgraph-schema_definition --> graphql;
elasticgraph-schema_definition --> graphql-c_parser;
elasticgraph-schema_definition --> rake;
class elasticgraph targetGemStyle;
class elasticgraph-support otherEgGemStyle;
Expand All @@ -149,13 +143,11 @@ graph LR;
class webrick externalGemCatStyle;
class elasticgraph-schema_artifacts otherEgGemStyle;
class graphql externalGemCatStyle;
class graphql-c_parser externalGemCatStyle;
click thor href "https://rubygems.org/gems/thor" "Open on RubyGems.org" _blank;
click rackup href "https://rubygems.org/gems/rackup" "Open on RubyGems.org" _blank;
click rake href "https://rubygems.org/gems/rake" "Open on RubyGems.org" _blank;
click webrick href "https://rubygems.org/gems/webrick" "Open on RubyGems.org" _blank;
click graphql href "https://rubygems.org/gems/graphql" "Open on RubyGems.org" _blank;
click graphql-c_parser href "https://rubygems.org/gems/graphql-c_parser" "Open on RubyGems.org" _blank;
```

### Datastore Adapters (2 gems)
Expand Down Expand Up @@ -227,7 +219,6 @@ graph LR;
elasticgraph-query_interceptor["eg-query_interceptor"];
elasticgraph-schema_artifacts["eg-schema_artifacts"];
elasticgraph-query_registry["eg-query_registry"];
graphql-c_parser["graphql-c_parser"];
rake["rake"];
elasticgraph-warehouse["eg-warehouse"];
elasticgraph-apollo --> elasticgraph-graphql;
Expand All @@ -242,7 +233,6 @@ graph LR;
elasticgraph-query_registry --> elasticgraph-graphql;
elasticgraph-query_registry --> elasticgraph-support;
elasticgraph-query_registry --> graphql;
elasticgraph-query_registry --> graphql-c_parser;
elasticgraph-query_registry --> rake;
elasticgraph-warehouse --> elasticgraph-support;
class elasticgraph-apollo targetGemStyle;
Expand All @@ -255,12 +245,10 @@ graph LR;
class elasticgraph-query_interceptor targetGemStyle;
class elasticgraph-schema_artifacts otherEgGemStyle;
class elasticgraph-query_registry targetGemStyle;
class graphql-c_parser externalGemCatStyle;
class rake externalGemCatStyle;
class elasticgraph-warehouse targetGemStyle;
click graphql href "https://rubygems.org/gems/graphql" "Open on RubyGems.org" _blank;
click apollo-federation href "https://rubygems.org/gems/apollo-federation" "Open on RubyGems.org" _blank;
click graphql-c_parser href "https://rubygems.org/gems/graphql-c_parser" "Open on RubyGems.org" _blank;
click rake href "https://rubygems.org/gems/rake" "Open on RubyGems.org" _blank;
```

Expand Down
2 changes: 2 additions & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ source "https://rubygems.org"
# Gems needed by the test suite and other CI checks.
group :development do
gem "aws_lambda_ric", "~> 3.1", ">= 3.1.3"
# graphql-c_parser is no longer a hard dependency, but we include it here for faster CI tests
gem "graphql-c_parser", "~> 1.1", ">= 1.1.3"
gem "benchmark-ips", "~> 2.14"
gem "coderay", "~> 1.1", ">= 1.1.3"
gem "factory_bot", "~> 6.5", ">= 6.5.6"
Expand Down
4 changes: 1 addition & 3 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ PATH
elasticgraph-datastore_core (= 1.1.1.pre)
elasticgraph-schema_artifacts (= 1.1.1.pre)
graphql (~> 2.5.18)
graphql-c_parser (~> 1.1, >= 1.1.3)

PATH
remote: elasticgraph-health_check
Expand Down Expand Up @@ -174,7 +173,6 @@ PATH
elasticgraph-graphql (= 1.1.1.pre)
elasticgraph-support (= 1.1.1.pre)
graphql (~> 2.5.18)
graphql-c_parser (~> 1.1, >= 1.1.3)
rake (~> 13.3, >= 13.3.1)

PATH
Expand All @@ -199,7 +197,6 @@ PATH
elasticgraph-schema_artifacts (= 1.1.1.pre)
elasticgraph-support (= 1.1.1.pre)
graphql (~> 2.5.18)
graphql-c_parser (~> 1.1, >= 1.1.3)
rake (~> 13.3, >= 13.3.1)

PATH
Expand Down Expand Up @@ -689,6 +686,7 @@ DEPENDENCIES
faker (~> 3.6)
filewatcher (~> 2.1)!
flatware-rspec (~> 2.3, >= 2.3.4)!
graphql-c_parser (~> 1.1, >= 1.1.3)
html-proofer (~> 5.2)
httpx (~> 1.7, >= 1.7.2)
irb (~> 1.16)
Expand Down
4 changes: 0 additions & 4 deletions elasticgraph-graphql/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,6 @@ graph LR;
graphql["graphql"];
elasticgraph-graphql --> graphql;
class graphql externalGemStyle;
graphql-c_parser["graphql-c_parser"];
elasticgraph-graphql --> graphql-c_parser;
class graphql-c_parser externalGemStyle;
elasticgraph-apollo["elasticgraph-apollo"];
elasticgraph-apollo --> elasticgraph-graphql;
class elasticgraph-apollo otherEgGemStyle;
Expand All @@ -52,7 +49,6 @@ graph LR;
class elasticgraph-schema_definition otherEgGemStyle;
click base64 href "https://rubygems.org/gems/base64" "Open on RubyGems.org" _blank;
click graphql href "https://rubygems.org/gems/graphql" "Open on RubyGems.org" _blank;
click graphql-c_parser href "https://rubygems.org/gems/graphql-c_parser" "Open on RubyGems.org" _blank;
```

## Usage
Expand Down
1 change: 0 additions & 1 deletion elasticgraph-graphql/elasticgraph-graphql.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ Gem::Specification.new do |spec|
spec.add_dependency "elasticgraph-datastore_core", ElasticGraph::VERSION
spec.add_dependency "elasticgraph-schema_artifacts", ElasticGraph::VERSION
spec.add_dependency "graphql", "~> 2.5.18"
spec.add_dependency "graphql-c_parser", "~> 1.1", ">= 1.1.3"

spec.add_development_dependency "elasticgraph-admin", ElasticGraph::VERSION
spec.add_development_dependency "elasticgraph-elasticsearch", ElasticGraph::VERSION
Expand Down
13 changes: 3 additions & 10 deletions elasticgraph-graphql/lib/elastic_graph/graphql.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
require "elastic_graph/graphql/config"
require "elastic_graph/constants"
require "elastic_graph/support/from_yaml_file"
require "elastic_graph/support/graphql_gem_loader"

module ElasticGraph
# The main entry point for ElasticGraph GraphQL handling. Instantiate this to get access to the
Expand Down Expand Up @@ -132,13 +133,7 @@ def datastore_query_builder
# @private
def graphql_gem_plugins
@graphql_gem_plugins ||= begin
require "graphql"
# As per https://graphql-ruby.org/language_tools/c_parser.html, loading the
# C parser causes the faster parser to be assigned as the `::GraphQL.default_parser`,
# providing greater efficiency.
#
# We load it here since this is where we load the GraphQL gem.
require "graphql/c_parser"
Support::GraphQLGemLoader.load

{
# We depend on this to avoid N+1 calls to the datastore.
Expand Down Expand Up @@ -268,9 +263,7 @@ def monotonic_clock
# at boot time instead of deferring dependency loading until we handle the first query. In other environments (such as tests),
# it's nice to load dependencies when needed.
def load_dependencies_eagerly
require "graphql"
require "graphql/c_parser"

Support::GraphQLGemLoader.load
::GraphQL.eager_load!

# run a simple GraphQL query to force load any dependencies needed to handle GraphQL queries
Expand Down
4 changes: 0 additions & 4 deletions elasticgraph-query_registry/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,10 @@ graph LR;
graphql["graphql"];
elasticgraph-query_registry --> graphql;
class graphql externalGemStyle;
graphql-c_parser["graphql-c_parser"];
elasticgraph-query_registry --> graphql-c_parser;
class graphql-c_parser externalGemStyle;
rake["rake"];
elasticgraph-query_registry --> rake;
class rake externalGemStyle;
click graphql href "https://rubygems.org/gems/graphql" "Open on RubyGems.org" _blank;
click graphql-c_parser href "https://rubygems.org/gems/graphql-c_parser" "Open on RubyGems.org" _blank;
click rake href "https://rubygems.org/gems/rake" "Open on RubyGems.org" _blank;
```

Expand Down
1 change: 0 additions & 1 deletion elasticgraph-query_registry/elasticgraph-query_registry.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ Gem::Specification.new do |spec|
spec.add_dependency "elasticgraph-graphql", ElasticGraph::VERSION
spec.add_dependency "elasticgraph-support", ElasticGraph::VERSION
spec.add_dependency "graphql", "~> 2.5.18"
spec.add_dependency "graphql-c_parser", "~> 1.1", ">= 1.1.3"
spec.add_dependency "rake", "~> 13.3", ">= 13.3.1"

spec.add_development_dependency "elasticgraph-elasticsearch", ElasticGraph::VERSION
Expand Down
5 changes: 3 additions & 2 deletions elasticgraph-query_registry/lib/elastic_graph/query_registry/query_validator.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@

require "elastic_graph/query_registry/variable_backward_incompatibility_detector"
require "elastic_graph/query_registry/variable_dumper"
require "graphql"
require "graphql/c_parser"
require "elastic_graph/support/graphql_gem_loader"

ElasticGraph::Support::GraphQLGemLoader.load

module ElasticGraph
module QueryRegistry
Expand Down
4 changes: 0 additions & 4 deletions elasticgraph-schema_definition/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,17 +30,13 @@ graph LR;
graphql["graphql"];
elasticgraph-schema_definition --> graphql;
class graphql externalGemStyle;
graphql-c_parser["graphql-c_parser"];
elasticgraph-schema_definition --> graphql-c_parser;
class graphql-c_parser externalGemStyle;
rake["rake"];
elasticgraph-schema_definition --> rake;
class rake externalGemStyle;
elasticgraph-local["elasticgraph-local"];
elasticgraph-local --> elasticgraph-schema_definition;
class elasticgraph-local otherEgGemStyle;
click graphql href "https://rubygems.org/gems/graphql" "Open on RubyGems.org" _blank;
click graphql-c_parser href "https://rubygems.org/gems/graphql-c_parser" "Open on RubyGems.org" _blank;
click rake href "https://rubygems.org/gems/rake" "Open on RubyGems.org" _blank;
```

Expand Down
1 change: 0 additions & 1 deletion elasticgraph-schema_definition/elasticgraph-schema_definition.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ Gem::Specification.new do |spec|
spec.add_dependency "elasticgraph-schema_artifacts", ElasticGraph::VERSION
spec.add_dependency "elasticgraph-support", ElasticGraph::VERSION
spec.add_dependency "graphql", "~> 2.5.18"
spec.add_dependency "graphql-c_parser", "~> 1.1", ">= 1.1.3"
spec.add_dependency "rake", "~> 13.3", ">= 13.3.1"

spec.add_development_dependency "elasticgraph-admin", ElasticGraph::VERSION
Expand Down
5 changes: 3 additions & 2 deletions ...ticgraph-schema_definition/lib/elastic_graph/schema_definition/schema_artifact_manager.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
require "did_you_mean"
require "elastic_graph/constants"
require "elastic_graph/schema_definition/json_schema_pruner"
require "elastic_graph/support/graphql_gem_loader"
require "elastic_graph/support/memoizable_data"
require "fileutils"
require "graphql"
require "graphql/c_parser"
require "tempfile"
require "yaml"

ElasticGraph::Support::GraphQLGemLoader.load

module ElasticGraph
module SchemaDefinition
# Manages schema artifacts. Note: not tested directly. Instead, the `RakeTasks` tests drive this class.
Expand Down
49 changes: 49 additions & 0 deletions elasticgraph-support/lib/elastic_graph/support/graphql_gem_loader.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Copyright 2024 - 2026 Block, Inc.
#
# Use of this source code is governed by an MIT-style
# license that can be found in the LICENSE file or at
# https://opensource.org/licenses/MIT.
#
# frozen_string_literal: true

module ElasticGraph
module Support
# Helper module to load the graphql gem and optionally graphql-c_parser for better performance.
# Prints a yellow warning to stderr when c_parser is unavailable (except on JRuby where C extensions don't work).
#
# @private
module GraphQLGemLoader
# ANSI escape code for yellow text
YELLOW = "\e[33m"
RESET = "\e[0m"

@warning_printed = false # : bool

# Loads the graphql gem and attempts to load graphql/c_parser.
# If c_parser is unavailable, prints a warning once (unless on JRuby).
def self.load
require "graphql"

begin
require "graphql/c_parser"
rescue LoadError
print_warning_once
end
end

def self.print_warning_once
return if @warning_printed || RUBY_ENGINE == "jruby"

@warning_printed = true
warn "#{YELLOW}[ElasticGraph] For better performance, add `graphql-c_parser` to your Gemfile. See: https://graphql-ruby.org/language_tools/c_parser.html#{RESET}"
end

# Resets warning state; only intended for use in tests.
def self.reset_warning_state!
@warning_printed = false
end

private_class_method :print_warning_once, :reset_warning_state!
end
end
end
14 changes: 14 additions & 0 deletions elasticgraph-support/sig/elastic_graph/support/graphql_gem_loader.rbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
module ElasticGraph
module Support
module GraphQLGemLoader
YELLOW: ::String
RESET: ::String

self.@warning_printed: bool

def self.load: () -> void
def self.print_warning_once: () -> void
def self.reset_warning_state!: () -> void
end
end
end
Loading