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.

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

Update java.ts - removing serverless link #204571

Merged
merged 5 commits into from
Jan 29, 2025
Merged

Update java.ts - removing serverless link #204571

merged 5 commits into from
Jan 29, 2025

Conversation

l-trotta
Copy link
Contributor

Replacing links to the serverless java client with the standard client following merging of the two.

@l-trotta l-trotta requested a review from a team as a code owner December 17, 2024 13:09
joemcelroy
joemcelroy reviewed Dec 18, 2024
View reviewed changes
x-pack/plugins/serverless_search/public/application/components/languages/java.ts Outdated
},
// Code Snippets,
installClient: `dependencies {
implementation 'co.elastic.clients:elasticsearch-java-serverless:1.0.0-20231031'
implementation 'com.fasterxml.jackson.core:jackson-databind:2.12.3'
implementation 'co.elastic.clients:elasticsearch-java:${elasticsearchVersion}'
Copy link
Member

Choose a reason for hiding this comment

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

is there a "latest" tag that we can use here? elasticsearchVersion variable doesn't exist

joemcelroy
joemcelroy reviewed Dec 18, 2024
View reviewed changes
x-pack/plugins/serverless_search/public/application/components/languages/java.ts Outdated
implementation 'co.elastic.clients:elasticsearch-java-serverless:1.0.0-20231031'
implementation 'com.fasterxml.jackson.core:jackson-databind:2.12.3'
implementation 'co.elastic.clients:elasticsearch-java:${elasticsearchVersion}'
implementation 'com.fasterxml.jackson.core:jackson-databind:${jacksonVersion}'
Copy link
Member

Choose a reason for hiding this comment

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

same with this jackson version

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 think the idea is to leave it to users to get the latest version? @swallez

Copy link
Member

Choose a reason for hiding this comment

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

Indeed, there is no concept of "latest" in Java dependency management. Users have to choose an explicit version, down to the minor. This is because Java dependency management appeared before semver existed.

So the use of variables in this PR is for us to avoid updating these files at every minor release by making them "versionless". Users will find the current version in the docs.

Copy link
Member

Choose a reason for hiding this comment

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

if left like this, it would actually throw an exception in the UI. ${elasticsearchVersion} syntax are variable interpolations which are missing.

I suggest you change these to $elasticsearchVersion if you want to give the appearance that they need to be replaced and can be rendered onto the UI.

image

I suggest that you run kibana also to check the display too!

swallez
swallez approved these changes Jan 28, 2025
View reviewed changes
Copy link
Member

@swallez swallez left a comment

Choose a reason for hiding this comment

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

LGTM

@l-trotta l-trotta force-pushed the serverless-java-client-fix branch from 4fb300d to 49a18f1 Compare January 29, 2025 08:56
@swallez swallez enabled auto-merge (squash) January 29, 2025 09:04
@l-trotta l-trotta force-pushed the serverless-java-client-fix branch 2 times, most recently from 21b8a1c to db1776d Compare January 29, 2025 11:22
joemcelroy
joemcelroy requested changes Jan 29, 2025
View reviewed changes
Copy link
Member

@joemcelroy joemcelroy left a comment

Choose a reason for hiding this comment

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

see comment on use of incorrect use of variable interpolation.

l-trotta and others added 3 commits January 29, 2025 12:48
@l-trotta
Update java.ts
7b10575 
Replacing links to the serverless java client with the standard client following merging of the two.
@swallez @l-trotta
Point to the root of the Java client repository
3006b01
@l-trotta
correct syntax
bfe743d
@l-trotta l-trotta force-pushed the serverless-java-client-fix branch from db1776d to bfe743d Compare January 29, 2025 11:49
@swallez
Copy link
Member

swallez commented Jan 29, 2025

@joemcelroy interpolation fixed. Please cross-check, and hopefully we can merge this time.

@swallez swallez merged commit 32e9a99 into main Jan 29, 2025
8 checks passed
@swallez swallez deleted the serverless-java-client-fix branch January 29, 2025 16:00
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #14 / Case Owner Selection renders all options
  • [job] [logs] Jest Tests #14 / Connector renders loading state correctly
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable deselects unknown users
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable does not show the same user in search results if it is already in the initial user profile mapping
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable does not shows initial empty search results on the list of users
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable remove all assignees
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable renders correctly
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable renders the assignees icons correctly
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable renders the icons correctly after selecting and deselecting assignees
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable renders the selected assignees label correctly
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable search and sorts alphabetically
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable selecting and deselecting a searched user does not show it after the user cleared the search
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable selects a searched user correctly
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable selects and unselects correctly assignees with multiple cases
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable selects and unselects correctly assignees with one case
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable selects unknown users
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable shows deselected users from the initial user profile mapping
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable shows the no matching component
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable shows unknown users
  • [job] [logs] Jest Tests #14 / EditAssigneesSelectable sort users alphabetically correctly

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
serverlessSearch 363.2KB 363.1KB -39.0B

History

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Jan 30, 2025
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

1 similar comment
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@l-trotta l-trotta added backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) backport:8.18 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Feb 6, 2025
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 7, 2025
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

5 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@joemcelroy
Copy link
Member

as this work only affects serverless, a backport isn't needed. Removing the label

@joemcelroy joemcelroy removed backport missing Added to PRs automatically when the are determined to be missing a backport. v9.0.0 backport:prev-minor Backport to (9.0) the previous minor version (i.e. one version back from main) backport:8.17 backport:8.18 labels Feb 14, 2025
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Feb 17, 2025
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

11 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create automatically backports add a backport:* label or prevent reminders by adding the backport:skip label.
You can also create backports manually by running node scripts/backport --pr 204571 locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joemcelroy joemcelroy joemcelroy approved these changes

@swallez swallez swallez approved these changes

Assignees
No one assigned
Labels
backport missing Added to PRs automatically when the are determined to be missing a backport. release_note:fix
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@l-trotta @swallez @elasticmachine @kibanamachine @joemcelroy