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

Upgrade Azure SDK #83884

Merged
merged 22 commits into from
Apr 13, 2022
Merged

Upgrade Azure SDK #83884

merged 22 commits into from
Apr 13, 2022

Conversation

fcofdez
Copy link
Contributor

@fcofdez fcofdez commented Feb 14, 2022

No description provided.

@fcofdez fcofdez added >non-issue :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.2.0 labels Feb 14, 2022
@arteam arteam self-requested a review February 16, 2022 11:26
Copy link
Contributor

@arteam arteam left a comment

Choose a reason for hiding this comment

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

LGTM! I've left a couple of stylistic comments

}
}

static class Resolver extends AddressResolverGroup<InetSocketAddress> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this resolver is the same as DefaultAddressResolverGroup.INSTANCE?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, this is a leftover from when I was tracking down the issue with the connection pool 👍

// The underlying http client uses this as part of the connection pool key,
// hence we need to use the same instance across all the client instances
// to avoid creating multiple connection pools.
record NioLoopResources(EventLoopGroup eventLoopGroup) implements LoopResources {
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel that we don't need to create a separate class for that, but can just use a lambda expression in the constructor

this.nioLoopResources = useNative -> eventLoopGroup

Copy link
Contributor

@arteam arteam left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks Francisco!

@weizijun
Copy link
Contributor

hi, @fcofdez @arteam , is the PR ok to merged, it will fix jaskson bad case, releated #80160

@arteam
Copy link
Contributor

arteam commented Mar 2, 2022

Ping @fcofdez, I think we are good to go with this PR.

@fcofdez
Copy link
Contributor Author

fcofdez commented Mar 2, 2022

ops! This fell through the cracks! I'll merge it!

@fcofdez
Copy link
Contributor Author

fcofdez commented Mar 2, 2022

@elasticmachine update branch

@fcofdez
Copy link
Contributor Author

fcofdez commented Mar 4, 2022

I have to fix #84231 before merging this one (it's related to the test infrastructure)

@elastic elastic deleted a comment from elasticmachine Mar 16, 2022
@jkakavas
Copy link
Member

jkakavas commented Mar 29, 2022

@fcofdez any chance to merge this soon so that it can make the next release ? I see #84231 is resolved. Also FYI 2.13.2.1 is the latest release since a few days, so we could upgrade to this instead ?

@fcofdez fcofdez force-pushed the upgrade-azure-sdk-returns branch from 1a1669f to 8cb9f55 Compare March 31, 2022 09:20
@fcofdez fcofdez changed the title Upgrade Azure SDK, Jackson and Snakeyaml Upgrade Azure SDK Mar 31, 2022
@rjernst
Copy link
Member

rjernst commented Apr 1, 2022

@fcofdez The example-plugins CI check failure can be ignored for now, it is due to the recent version bump. This PR should be good to merge. I think it should go back to 8.2 and 7.17.

@fcofdez fcofdez merged commit 351d234 into elastic:master Apr 13, 2022
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Apr 13, 2022
…n/elasticsearch into datastream-reuse-pipeline-source

* 'datastream-reuse-pipeline-source' of github.com:weizijun/elasticsearch: (28 commits)
  Add JDK 19 to Java testing matrix
  [ML] add nlp config update serialization tests (elastic#85867)
  [ML] A text categorization aggregation that works like ML categorization (elastic#80867)
  [ML] Fix serialisation of text embedding updates (elastic#85863)
  TSDB: fix wrong initial value of tsidOrd in TimeSeriesIndexSearcher (elastic#85713)
  Enforce external id uniqueness during DesiredNode construction (elastic#84227)
  Fix Intellij integration (elastic#85866)
  Upgrade Azure SDK to version 12.14.4 (elastic#83884)
  [discovery-gce] Fix initialisation of transport in FIPS mode (elastic#85817)
  Remove unnecessary docs/changelog/85534.yaml
  Prevent ThreadContext header leak when sending response (elastic#68649)
  Add support for impact_areas to health impacts  (elastic#85830)
  Reduce port range re-use in tests (elastic#85777)
  Fix TranslogTests#testStats (elastic#85828)
  Remove hppc from cat allocation api (elastic#85842)
  Fix BuildTests serialization (elastic#85827)
  Use urgent priority for node shutdown cluster state update (elastic#85838)
  Remove Task classes from HLRC (elastic#85835)
  Remove unused migration classes (elastic#85834)
  Remove uses of Charset name parsing (elastic#85795)
  ...
@weizijun
Copy link
Contributor

hi, @fcofdez , I see you are revert the jackson version. Can you tell what's the revert reason? There is a bug in jackson, and need to be upgrade. Here is the close issue: #80160

@fcofdez
Copy link
Contributor Author

fcofdez commented Apr 25, 2022

Jackson has been upgraded separately in this PR #86051 cc @weizijun

@weizijun
Copy link
Contributor

Jackson has been upgraded separately in this PR #86051 cc @weizijun

Yeah, Nik told me about this PR, thanks!

@nik9000
Copy link
Member

nik9000 commented Apr 25, 2022 via email

fcofdez added a commit that referenced this pull request Apr 26, 2022
In this version jackson was upgraded to the version 2.13.2.1 that we were using
before. See https://github.com/Azure/azure-sdk-for-java/releases/tag/azure-core_1.27.0

Without the upgrade, the third party SAS tests were failing, once upgraded the tests pass again,
I think that we were hitting some incompatibility with the jackson version that we were
providing.

Relates #83884
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants