Skip to content

Use JSON ingestion as schema definition extension#1205

Merged
jwils merged 6 commits into
mainfrom
joshuaw/json-ingestion-default-extension
Jun 16, 2026
Merged

Use JSON ingestion as schema definition extension#1205
jwils merged 6 commits into
mainfrom
joshuaw/json-ingestion-default-extension

Conversation

@jwils

@jwils jwils commented May 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Why

Complete the extraction by loading JSON ingestion through the schema-definition extension API and removing the duplicated core JSON Schema implementation.

What

  • Wire ElasticGraph::JSONIngestion::SchemaDefinition::APIExtension into app, template, doctest, and spec-helper schema definition setup
  • Move JSON Schema artifact/results behavior out of core schema_definition
  • Remove JSON Schema field/type/index logic from core classes now provided by the extension
  • Keep JSON schema version enforcement configurable through the JSON ingestion extension
  • Update the generated JSON ingestion dependency diagram

Risk Assessment

Medium to high - this is the behavior-transfer layer, but it sits after the move/namespace/wrapper/wiring PRs so the remaining diff is mostly deletion and wiring.

References

Stack

Current PR is marked with ->.

@jwils jwils mentioned this pull request May 20, 2026
Merged
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from f9fb615 to edc42c3 Compare May 20, 2026 20:03
@jwils
jwils force-pushed the joshuaw/json-ingestion-extension-modules branch 2 times, most recently from 024c4ef to d8c3c70 Compare May 20, 2026 20:15
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from edc42c3 to 62013f0 Compare May 20, 2026 20:15
@jwils
jwils force-pushed the joshuaw/json-ingestion-extension-modules branch from d8c3c70 to b78686f Compare May 21, 2026 02:49
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch 2 times, most recently from 1eec3a6 to f13769d Compare May 21, 2026 13:43
@jwils
jwils force-pushed the joshuaw/json-ingestion-extension-modules branch from b78686f to 0b3844f Compare May 21, 2026 13:43
@jwils
jwils marked this pull request as ready for review May 22, 2026 18:24
@myronmarston myronmarston mentioned this pull request May 22, 2026
Merged
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from f13769d to ee7fe58 Compare May 22, 2026 22:41
@jwils
jwils force-pushed the joshuaw/json-ingestion-extension-modules branch from 0b3844f to 65f9a60 Compare May 22, 2026 22:41
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from ee7fe58 to 4170e75 Compare May 24, 2026 13:33
@jwils
jwils force-pushed the joshuaw/json-ingestion-extension-modules branch from 65f9a60 to 6be803f Compare May 24, 2026 13:33
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from 4170e75 to a49a787 Compare May 24, 2026 14:09
@jwils
jwils force-pushed the joshuaw/json-ingestion-extension-modules branch from 6be803f to 159afa8 Compare May 24, 2026 14:09
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from a49a787 to 68bd00b Compare May 24, 2026 14:24
@jwils
jwils force-pushed the joshuaw/json-ingestion-extension-modules branch from 159afa8 to 29e7e8e Compare May 24, 2026 14:24
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from 68bd00b to d4a6dcf Compare May 24, 2026 14:51
@jwils
jwils force-pushed the joshuaw/json-ingestion-extension-modules branch 2 times, most recently from 31d7828 to 87c88c1 Compare May 24, 2026 15:00
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from d4a6dcf to 05163d1 Compare May 24, 2026 15:00
@jwils
jwils force-pushed the joshuaw/json-ingestion-extension-modules branch from 87c88c1 to e99c65f Compare May 24, 2026 15:36
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from 05163d1 to 97557eb Compare May 24, 2026 15:36
@jwils
jwils force-pushed the joshuaw/json-ingestion-api-polish branch from 5b67eda to 28d3b58 Compare May 28, 2026 19:01
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from 82a633c to 16b791e Compare May 28, 2026 19:01
@jwils
jwils force-pushed the joshuaw/json-ingestion-api-polish branch from 28d3b58 to 4f85649 Compare May 28, 2026 19:13
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch 3 times, most recently from 8f4a490 to ee6f074 Compare May 28, 2026 20:36
@jwils
jwils force-pushed the joshuaw/json-ingestion-api-polish branch from 4f85649 to fd4651f Compare May 30, 2026 14:07
@jwils
jwils force-pushed the joshuaw/json-ingestion-default-extension branch from ee6f074 to 5984788 Compare May 30, 2026 14:07
@jwils
jwils force-pushed the joshuaw/json-ingestion-api-polish branch from fd4651f to c1898a3 Compare May 30, 2026 14:26
This was referenced Jun 1, 2026
Merged
Merged
Merged
Merged
Merged
Merged
Open
myronmarston
myronmarston requested changes Jun 5, 2026
View reviewed changes

@myronmarston myronmarston left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not done reviewing but wanted to submit what I had so far.

Comment thread config/site/support/doctest_helper.rb Outdated
Comment thread config/site/support/doctest_helper.rb Outdated
Comment thread config/site/support/doctest_helper.rb Outdated
Comment thread config/site/support/doctest_helper.rb Outdated
Comment thread elasticgraph-apollo/spec/unit/elastic_graph/apollo/apollo_directives_spec.rb
Comment thread elasticgraph-apollo/README.md
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_type_info.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_type_info.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/mixins/has_type_info.rb Outdated
Comment thread ...icgraph-schema_definition/lib/elastic_graph/schema_definition/schema_elements/scalar_type.rb Outdated
myronmarston
myronmarston reviewed Jun 9, 2026
View reviewed changes

@myronmarston myronmarston left a comment •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Next round of feedback (still haven't made it through the whole PR given how big it is!).

Regarding doc_comment--definitely feel free to keep it on the eg-schema_definition objects if there's not a cleaner way to pass it through to the JSON schema logic that uses it.

Comment thread config/site/Rakefile Outdated
Comment thread elasticgraph-apollo/README.md
Comment thread ...n_ingestion/lib/elastic_graph/json_ingestion/schema_definition/indexing/field_type/scalar.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/field_type/enum.rb Outdated
Comment thread ...ticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/field_type/object.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/field_type/union.rb
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/field.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/field_reference.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/field_reference.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/indexing/field_reference.rb Outdated
myronmarston
myronmarston reviewed Jun 9, 2026
View reviewed changes
Comment thread Rakefile Outdated
myronmarston
myronmarston requested changes Jun 10, 2026
View reviewed changes
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb Outdated
Comment thread elasticgraph-schema_definition/lib/elastic_graph/schema_definition/api.rb Outdated
myronmarston
myronmarston approved these changes Jun 10, 2026
View reviewed changes

@myronmarston myronmarston left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done!

Comment thread ...cgraph-schema_definition/spec/integration/elastic_graph/schema_definition/rake_tasks_spec.rb
Comment thread spec_support/lib/elastic_graph/spec_support/schema_definition_helpers.rb
This was referenced Jun 11, 2026
Merged
Closed
jwils added 6 commits June 16, 2026 13:13
@jwils
Use JSON ingestion as schema definition extension
f6ec255
@jwils
Address review feedback on JSON ingestion extension PR
2ff6753 
- Make core `json_schema_version`, `enforce_json_schema_version`, and
  `json_schema_strictness` raise a clear error pointing at
  `elasticgraph-json_ingestion` instead of silently no-oping, so apps that
  upgrade without adding the extension module fail fast rather than silently
  dumping stale JSON schema artifacts. A new
  `API#supports_json_schema_versioning?` predicate lets test support, the
  doctest helper, and other callers detect whether an extension provides the
  real behavior.
- Remove the `State#json_schema_version` nil stub now that nothing needs it.
- Restore the explanatory comment for why core infers no grouping placeholder
  for `long`/`unsigned_long` mappings.
- Drop the `:nocov:` around `Indexing::Index#initialize`'s block handling and
  cover the no-block path with a test instead.
- Inline `validate_json_schema_configuration!` into
  `finalize_json_schema_configuration!` (its only caller), removing a
  redundant `graphql_only?` guard.
- Deduplicate the optional-extension bootstrap between `spec_helper.rb` and
  `schema_definition_helpers.rb`, and collapse the `define_schema` overrides
  to keyword splats (also fixing `reload_schema_artifacts` being silently
  dropped).
- Revert the unrelated `profiling.rb` change and the `api.rb` require
  reordering.
- Drop redundant `ElasticGraph::` prefixes inside `module ElasticGraph`.
- Add the JSON ingestion extension to the example Rakefiles that have their
  own `Rakefile` (music, schema_customization_rake_tasks).
@jwils
Add JSON ingestion extension to elasticgraph-local acceptance spec
cb87039 
The acceptance spec drives the repo's own `config/schema.rb` (which calls
`schema.json_schema_version`) through `Local::RakeTasks`, so it needs the
JSON ingestion extension now that the core API raises without it.
@jwils
Remove whitespace-only churn from README example diffs
53ecfa8 
The embedded Rakefile diffs in the apollo and warehouse READMEs wrote blank
context lines as a single space (rendering as a no-op remove/re-add of a blank
line plus trailing whitespace), and the warehouse example removed and re-added
a blank line instead of just inserting the new extension line.
@jwils
Revert RBS reordering not needed for type checking
680cf75 
Keeps the api.rbs and state.rbs diffs minimal: the JSON schema method
signatures stay in their original positions and the `output` accessor keeps
its original position, with only the genuinely new signatures added.
@jwils
Drop JSON schema definition methods from core entirely
943eb53 
Per review feedback: `elasticgraph-schema_definition` should not know about
JSON schema at all, so `json_schema_version`, `enforce_json_schema_version`,
and `json_schema_strictness` no longer exist on the core API (calling them
without `elasticgraph-json_ingestion` raises `NoMethodError`). Test support
and the doctest helper feature-detect the extension with `respond_to?`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@myronmarston myronmarston myronmarston approved these changes
@ayousufi ayousufi Awaiting requested review from ayousufi
@BrianSigafoos-SQ BrianSigafoos-SQ Awaiting requested review from BrianSigafoos-SQ BrianSigafoos-SQ is a code owner
@bsorbo bsorbo Awaiting requested review from bsorbo bsorbo is a code owner
@jwondrusch jwondrusch Awaiting requested review from jwondrusch jwondrusch is a code owner
@marcdaniels-toast marcdaniels-toast Awaiting requested review from marcdaniels-toast marcdaniels-toast is a code owner
@ellisandrews-toast ellisandrews-toast Awaiting requested review from ellisandrews-toast ellisandrews-toast is a code owner
@rossroberts-toast rossroberts-toast Awaiting requested review from rossroberts-toast rossroberts-toast is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jwils @myronmarston