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

Include Schemas in NodeContext #15988

Merged
merged 3 commits into from
May 22, 2024
Merged

Include Schemas in NodeContext #15988

merged 3 commits into from
May 22, 2024

Conversation

jeeminso
Copy link
Contributor

@jeeminso jeeminso commented May 14, 2024

Summary of the changes / Why this improves CrateDB

Introduce Schemas field in NodeContext which requires manual creation of the Schemas instance(moving away from the ES injector) and also all of its fields.

All the classes with the singleton/inject annotations removed are now manually created but some may still be injected to other instances.

Checklist

  • Added an entry in the latest docs/appendices/release-notes/<x.y.0>.rst for user facing changes
  • Updated documentation & sql_features table for user facing changes
  • Touched code is covered by tests
  • CLA is signed
  • This does not contain breaking changes, or if it does:
    • It is released within a major release
    • It is recorded in the latest docs/appendices/release-notes/<x.y.0>.rst
    • It was marked as deprecated in an earlier release if possible
    • You've thought about the consequences and other components are adapted
      (E.g. AdminUI)

@jeeminso jeeminso force-pushed the jeeminso/schemas-nodectx branch 10 times, most recently from 2bf66e8 to 7d2ce80 Compare May 16, 2024 12:36
@jeeminso jeeminso marked this pull request as ready for review May 17, 2024 09:25
this.roles = roles;
this.createSchemas = createSchemas;
schemas();
Copy link
Member

Choose a reason for hiding this comment

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

If this works eagerly couldn't we make it final and remove the lazy initialization logic?

Schemas and all of its fields are no longer created by ES injector.

The classes with the singleton/inject annotations removed are now
manually created but some may still be injected to other instances
by the injector.

Co-authored-by: Mathias Fussenegger <f.mathias@zignar.net>
@jeeminso jeeminso added the ready-to-merge Let Mergify merge the PR once approved and checks pass label May 22, 2024
@mergify mergify bot merged commit eddf12c into master May 22, 2024
14 checks passed
@mergify mergify bot deleted the jeeminso/schemas-nodectx branch May 22, 2024 15:42
@seut seut added the v/5.7 label May 28, 2024
mfussenegger added a commit that referenced this pull request May 29, 2024
Follow up to #15988, which removed
the setter but didn't update the comment on the field.
mfussenegger added a commit that referenced this pull request May 29, 2024
Follow up to #15988, which removed
the setter but didn't update the comment on the field.

Also removes the unused client
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge Let Mergify merge the PR once approved and checks pass v/5.7
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants