Skip to content

[Onboarding] Small fixes from QA #196178

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

Merged
joemcelroy merged 2 commits into from
Oct 15, 2024
Merged

[Onboarding] Small fixes from QA #196178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3b2fed
update code samples
joemcelroy Oct 14, 2024
f580034
update
joemcelroy Oct 14, 2024
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
6 changes: 3 additions & 3 deletions x-pack/plugins/search_indices/public/code_examples/javascript.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export const JAVASCRIPT_INFO: CodeLanguage = {
codeBlockLanguage: 'javascript',
};

const SERVERLESS_INSTALL_CMD = `npm install @elastic/elasticsearch-serverless`;
const SERVERLESS_INSTALL_CMD = `npm install @elastic/elasticsearch`;
Copy link
Copy Markdown
Member Author

@joemcelroy joemcelroy Oct 14, 2024

Choose a reason for hiding this comment

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

we made a decision to use the elasticsearch client vs the elasticsearch-serverless client even for serverless. cc @miguelgrinberg

Copy link
Copy Markdown
Contributor

@miguelgrinberg miguelgrinberg Oct 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I'm 👍 but we also need @pquentin and @JoshMock to comment.

Copy link
Copy Markdown
Member

@JoshMock JoshMock Oct 14, 2024

Choose a reason for hiding this comment

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

If this is just for generating code snippets presented to Serverless customers, I don't see an issue. 👍

For a second I thought this was about actually using the Serverless client in the Kibana codebase, which was discussed in #169674 and is currently blocked.

Copy link
Copy Markdown
Member Author

@joemcelroy joemcelroy Oct 14, 2024

Choose a reason for hiding this comment

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

Yes, just generating code snippets. It's mainly about what we should advise developers to use for interfacing with serverless. When I spoke with @miguelgrinberg, we decided that we want customers to use Elasticsearch client for both stack and serverless. Just double-checking here to ensure that this hasn't changed.

Also, it would be good to update the code exporter library with this direction. I'm hoping to use the code exporter for these examples in the future instead.

Copy link
Copy Markdown
Member

@pquentin pquentin Oct 15, 2024

Choose a reason for hiding this comment

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

Yes, just generating code snippets. It's mainly about what we should advise developers to use for interfacing with serverless. When I spoke with @miguelgrinberg, we decided that we want customers to use Elasticsearch client for both stack and serverless. Just double-checking here to ensure that this hasn't changed.

For the foreseeable future, the Serverless API will remain a subset of the Elasticsearch API, with the caveat that features usually land in Serverless weeks before they go in an Elasticsearch release. I think I'm lacking context here, but for "developers that interface with Serverless", we should likely recommend the elasticsearch-serverless client, as:

  1. it won't encourage you to use APIs that don't exist in Serverless,
  2. it will contain the APIs that are in Serverless but not in the Stack releases yet.

But it's not that simple:

  • The elasticsearch client will work on Serverless, after all. And we care less about code completion for examples.
  • I don't think the elasticsearch-serverless client supports HTTP, which means it can't be used against start-local. Likewise, it does not support like node sniffing which might be useful to use conditionally if you're migrating from a local cluster to Serverless.
  • We initially wanted to have a "serverless mode" in the elasticsearch client to restrict the APIs and parameters to the subset that worked everywhere, but since Serverless is only a subset, that was not needed.
  • There are ongoing discussions to merge the two clients, but it's unclear how we can make that work yet due to the very different release schedules.

For all those reasons, it can make sense to use the elasticsearch client here. In other words, LGTM!

Also, it would be good to update the code exporter library with this direction. I'm hoping to use the code exporter for these examples in the future instead.

What is the code exporter library? request-converter, which is used in Dev Tools to export Python and JS code, uses the elasticsearch client: https://github.com/elastic/request-converter/blob/main/src/exporters/javascript.tpl

Copy link
Copy Markdown
Member Author

@joemcelroy joemcelroy Oct 15, 2024

Choose a reason for hiding this comment

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

Sorry yes, i meant the request-converter. Great that always uses elasticsearch client!


export const JavascriptServerlessCreateIndexExamples: CreateIndexLanguageExamples = {
default: {
Expand All @@ -28,7 +28,7 @@ export const JavascriptServerlessCreateIndexExamples: CreateIndexLanguageExample
elasticsearchURL,
apiKey,
indexName,
}) => `import { Client } from "@elastic/elasticsearch-serverless"
}) => `import { Client } from "@elastic/elasticsearch"

const client = new Client({
node: '${elasticsearchURL}',
Expand All @@ -47,7 +47,7 @@ client.indices.create({
elasticsearchURL,
apiKey,
indexName,
}) => `import { Client } from "@elastic/elasticsearch-serverless"
}) => `import { Client } from "@elastic/elasticsearch"

const client = new Client({
node: '${elasticsearchURL}',
Expand Down
43 changes: 19 additions & 24 deletions x-pack/plugins/search_indices/public/code_examples/python.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const PYTHON_INFO: CodeLanguage = {
codeBlockLanguage: 'python',
};

const SERVERLESS_PYTHON_INSTALL_CMD = 'pip install elasticsearch-serverless';
const SERVERLESS_PYTHON_INSTALL_CMD = 'pip install elasticsearch';

export const PythonServerlessCreateIndexExamples: CreateIndexLanguageExamples = {
default: {
Expand All @@ -32,7 +32,7 @@ export const PythonServerlessCreateIndexExamples: CreateIndexLanguageExamples =
elasticsearchURL,
apiKey,
indexName,
}: CodeSnippetParameters) => `from elasticsearch-serverless import Elasticsearch
}: CodeSnippetParameters) => `from elasticsearch import Elasticsearch

client = Elasticsearch(
"${elasticsearchURL}",
Expand All @@ -49,21 +49,21 @@ client.indices.create(
elasticsearchURL,
apiKey,
indexName,
}: CodeSnippetParameters) => `from elasticsearch-serverless import Elasticsearch
}: CodeSnippetParameters) => `from elasticsearch import Elasticsearch

client = Elasticsearch(
"${elasticsearchURL}",
api_key="${apiKey ?? API_KEY_PLACEHOLDER}"
"${elasticsearchURL}",
api_key="${apiKey ?? API_KEY_PLACEHOLDER}"
)

client.indices.create(
index="${indexName ?? INDEX_PLACEHOLDER}"
mappings={
"properties": {
"vector": {"type": "dense_vector", "dims": 3 },
"text": {"type": "text"}
}
}
index="${indexName ?? INDEX_PLACEHOLDER}",
mappings={
"properties": {
"vector": {"type": "dense_vector", "dims": 3 },
"text": {"type": "text"}
}
}
)`,
},
};
Expand All @@ -72,7 +72,7 @@ const serverlessIngestionCommand: IngestCodeSnippetFunction = ({
apiKey,
indexName,
sampleDocument,
}) => `from elasticsearch-serverless import Elasticsearch, helpers
}) => `from elasticsearch import Elasticsearch, helpers

client = Elasticsearch(
"${elasticsearchURL}",
Expand All @@ -93,25 +93,20 @@ const serverlessUpdateMappingsCommand: IngestCodeSnippetFunction = ({
apiKey,
indexName,
mappingProperties,
}) => `from elasticsearch-serverless import Elasticsearch
}) => `from elasticsearch import Elasticsearch

client = Elasticsearch(
"${elasticsearchURL}",
api_key="${apiKey ?? API_KEY_PLACEHOLDER}"
"${elasticsearchURL}",
api_key="${apiKey ?? API_KEY_PLACEHOLDER}"
)

index_name = "${indexName}"

mappings = ${JSON.stringify({ properties: mappingProperties }, null, 4)}

update_mapping_response = client.indices.put_mapping(index=index_name, body=mappings)

# Print the response
print(update_mapping_response)

# Verify the mapping
mapping = client.indices.get_mapping(index=index_name)
print(mapping)`;
mapping_response = client.indices.put_mapping(index=index_name, body=mappings)
print(mapping_response)
`;

export const PythonServerlessVectorsIngestDataExample: IngestDataCodeDefinition = {
installCommand: SERVERLESS_PYTHON_INSTALL_CMD,
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugins/search_indices/public/components/shared/code_sample.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ export const CodeSample = ({ id, title, language, code, onCodeCopyClick }: CodeS
paddingSize="m"
isCopyable
transparentBackground
css={{
'*::selection': {
backgroundColor: 'rgba(255, 255, 255, 0.2)',
},
}}
>
{code}
</EuiCodeBlock>
Expand Down