Skip to content

[EIS] Validate EIS Gateway URL if set #114600

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

Jump to bottom
Merged
merged 6 commits into from
Oct 15, 2024
Merged

[EIS] Validate EIS Gateway URL if set #114600

merged 6 commits into from
Oct 15, 2024

Conversation

@timgrein
Copy link
Contributor

This PR adds a Setting.Validator to validate the EIS Gateway URL set through xpack.inference.eis.gateway.url. If the URL is not set (for example, if you're not using EIS) we skip validation.

@timgrein timgrein added >non-issue :ml Machine learning Team:ML Meta label for the ML team v9.0.0 labels Oct 11, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

@timgrein
Copy link
Contributor Author

@elasticmachine update branch

@timgrein timgrein added the :SearchOrg/Inference Label for the Search Inference team label Oct 14, 2024
@elasticsearchmachine elasticsearchmachine added Team:SearchOrg Meta label for the Search Org (Enterprise Search) Team:Search - Inference labels Oct 14, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/search-inference-team (Team:Search - Inference)

@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/search-eng (Team:SearchOrg)

vidok
vidok reviewed Oct 14, 2024
View reviewed changes
...java/org/elasticsearch/xpack/inference/services/elastic/ElasticInferenceServiceSettings.java
@Override
public void validate(String value) {
if (Objects.isNull(value) || value.isEmpty()) {
// No validation needed, if eis-gateway URL is not set
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to write to the logs that EIS is not enabled?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've checked what gets logged on startup and it feels like it's rather uncommon to log something as specific as this (at least on the INFO level). There are only a few plugins logging, whether they're enabled (on a whole plugin level) like APM and security. But I think it probably doesn't cause any harm to log it on DEBUG level, so adding it with Add debug log, whether eis-gateway URL is set or not.

@timgrein timgrein removed the v8.16.0 label Oct 15, 2024
@timgrein
Copy link
Contributor Author

@elasticmachine update branch

@timgrein timgrein merged commit 39168e1 into elastic:main Oct 15, 2024
16 checks passed
kderusso pushed a commit to kderusso/elasticsearch that referenced this pull request Oct 15, 2024
@timgrein @kderusso
[EIS] Validate EIS Gateway URL if set (elastic#114600)
bbfc390
mark-vieira added a commit to mark-vieira/elasticsearch that referenced this pull request Oct 15, 2024
@mark-vieira
Revert "[EIS] Validate EIS Gateway URL if set (elastic#114600)"
4cd2acd 
This reverts commit 39168e1.
@mark-vieira
Copy link
Contributor

This is manifesting in serverless with the following:

[2024-10-15T21:34:26,251][ERROR][o.e.b.Elasticsearch      ] [es-es-index-5d88674cbc-bs9mw] fatal exception while booting Elasticsearch java.lang.IllegalArgumentException: [null] is not a valid URI scheme for the setting [xpack.inference.eis.gateway.url]. Use one of [http,https]
	at org.elasticsearch.inference@9.0.0/org.elasticsearch.xpack.inference.services.elastic.ElasticInferenceServiceSettings$EisGatewayURLValidator.validate(ElasticInferenceServiceSettings.java:60)
	at org.elasticsearch.inference@9.0.0/org.elasticsearch.xpack.inference.services.elastic.ElasticInferenceServiceSettings$EisGatewayURLValidator.validate(ElasticInferenceServiceSettings.java:37)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.common.settings.Setting.get(Setting.java:557)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.common.settings.Setting.get(Setting.java:530)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:604)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:510)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:480)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.common.settings.AbstractScopedSettings.validate(AbstractScopedSettings.java:450)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.common.settings.SettingsModule.<init>(SettingsModule.java:133)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.common.settings.SettingsModule.<init>(SettingsModule.java:51)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.node.NodeConstruction.validateSettings(NodeConstruction.java:527)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.node.NodeConstruction.prepareConstruction(NodeConstruction.java:277)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.node.Node.<init>(Node.java:200)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.bootstrap.Elasticsearch$2.<init>(Elasticsearch.java:240)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.bootstrap.Elasticsearch.initPhase3(Elasticsearch.java:240)
	at org.elasticsearch.server@9.0.0/org.elasticsearch.bootstrap.Elasticsearch.main(Elasticsearch.java:75)

elasticsearchmachine pushed a commit that referenced this pull request Oct 15, 2024
@mark-vieira
Revert "[EIS] Validate EIS Gateway URL if set (#114600)" (#114867)
8ccfb22 
This reverts commit 39168e1.
@mark-vieira
Copy link
Contributor

FYI, I've reverted this for now in #114867

@timgrein
Copy link
Contributor Author

@mark-vieira thank you Mark!

georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Oct 25, 2024
@timgrein @georgewallace
[EIS] Validate EIS Gateway URL if set (elastic#114600)
30825f2
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Oct 25, 2024
@mark-vieira @georgewallace
Revert "[EIS] Validate EIS Gateway URL if set (elastic#114600)" (elas…
432a3ab 
…tic#114867)

This reverts commit 39168e1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vidok vidok vidok approved these changes

@prwhelan prwhelan prwhelan approved these changes

Assignees

No one assigned

Labels

:ml Machine learning >non-issue :SearchOrg/Inference Label for the Search Inference team Team:ML Meta label for the ML team Team:Search - Inference Team:SearchOrg Meta label for the Search Org (Enterprise Search) v9.0.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@timgrein @elasticsearchmachine @mark-vieira @vidok @prwhelan @elasticmachine