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 jackson and azure #80160

Closed
wants to merge 5 commits into from
Closed

Update jackson and azure #80160

wants to merge 5 commits into from

Conversation

nik9000
Copy link
Member

@nik9000 nik9000 commented Nov 1, 2021

This grabs a new version of jackson and snakeyaml to get some bug fixes.
It also updates azure to stay compatible with the new jackson.

Closes #66555
Closes #67214
Closes #80142

@nik9000
Copy link
Member Author

nik9000 commented Nov 1, 2021

There were other changes the last time we tried to update azure - https://github.com/elastic/elasticsearch/pull/73837/files#diff-0a3347d8c82d7507fa4507d2fde0f5feb4fa718a0d5025a249afbcde5621c555 - but the tests don't fail and I don't know what they were for.

@nik9000 nik9000 added :Core/Infra/Core Core issues without another label :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Nov 1, 2021
@elasticmachine elasticmachine added Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. Team:Core/Infra Meta label for core/infra team labels Nov 1, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@nik9000 nik9000 requested a review from fcofdez November 1, 2021 19:56
@nik9000 nik9000 added the test-windows Trigger CI checks on Windows label Nov 1, 2021
@nik9000
Update jackson and azure
2820d0a 
This grabs a new version of jackson and snakeyaml to get some bug fixes.
It also updates azure to stay compatible with the new jackson.

Closes elastic#66555
Closes elastic#67214
Closes elastic#80142
@nik9000
No databind
6b1da9d
@fcofdez
Copy link
Contributor

fcofdez commented Nov 2, 2021

Sadly we are still blocked by this issue Azure/azure-sdk-for-java#22042 I've pinged the team several times but it seems like there hasn't been any progress on that

@nik9000
Copy link
Member Author

nik9000 commented Nov 2, 2021

There were other changes the last time we tried to update azure - https://github.com/elastic/elasticsearch/pull/73837/files#diff-0a3347d8c82d7507fa4507d2fde0f5feb4fa718a0d5025a249afbcde5621c555 - but the tests don't fail and I don't know what they were for.

Adding these back helped the broken tests.

Sadly we are still blocked by this issue Azure/azure-sdk-for-java#22042 I've pinged the team several times but it seems like there hasn't been any progress on that

@fcofdez Oh! Is there any way we can get just jackson upgraded? I really do want to get us unstuck and I don't know anything about Azure.

@nik9000
Copy link
Member Author

nik9000 commented Nov 2, 2021

@fcofdez Oh! Is there any way we can get just jackson upgraded? I really do want to get us unstuck and I don't know anything about Azure.

Do we have any tests that'd fail on upgrade that I can trigger btw?

@grcevski
Copy link
Contributor

grcevski commented Nov 2, 2021

Sadly we are still blocked by this issue Azure/azure-sdk-for-java#22042 I've pinged the team several times but it seems like there hasn't been any progress on that

I know few folks on a related team, I'll reach out.

@fcofdez
Copy link
Contributor

fcofdez commented Nov 3, 2021

Do we have any tests that'd fail on upgrade that I can trigger btw?

The CI job that runs the problematic test is elastic+elasticsearch+{branch}+multijob+third-party-tests-azure-sas but I don't know how to trigger those in a PR.

@nik9000
Copy link
Member Author

nik9000 commented Nov 3, 2021

** elasticsearch-ci/part-1-windows ** — Build finished.

This failure is the same one we had last time we tried to update:

»  Caused by: java.security.AccessControlException: access denied ("java.io.FilePermission" "C:\Windows\system32\drivers\etc\hosts" "read")

I really don't know anything about the Azure deployment. I'd like to hand this upgrade off to someone that does. I'm many layers deep from the code I was trying to write in the first place and there are much better experts over here.

@fcofdez
Copy link
Contributor

fcofdez commented Nov 3, 2021

@nik9000 I can take care of this once the bug in the azure SDK is fixed

@nik9000
Copy link
Member Author

nik9000 commented Nov 3, 2021

@nik9000 I can take care of this once the bug in the azure SDK is fixed

Thanks so much. I'm way out of my depth here. Or, at least, it'd take me a lot of reading to catch up.

@nik9000 nik9000 mentioned this pull request Nov 3, 2021
Open
@weizijun
Copy link
Contributor

weizijun commented Dec 8, 2021

hi, @nik9000 , is it necessary to update jackson first? as the badcase in exclude exist in the current version?

@nik9000
Copy link
Member Author

nik9000 commented Dec 8, 2021

hi, @nik9000 , is it necessary to update jackson first? as the badcase in exclude exist in the current version?

For now at least these are stuck together due to a breaking change in Jackson that breaks azure. It's pretty deep so I can see how the folks that made the change didn't think of it as a version change thing.

I think we're waiting on @ChrisHegarty's contacts at Azure to update to the newer version of jackson.

@ChrisHegarty
Copy link
Contributor

I think we're waiting on @ChrisHegarty's contacts at Azure to update to the newer version of jackson.

That would be @grcevski's contact. :-)

@fcofdez
Copy link
Contributor

fcofdez commented Dec 8, 2021

The bug was finally fixed Azure/azure-sdk-for-java#22042 once they release a new version with the fix, I'll upgrade the libraries.

@weizijun weizijun mentioned this pull request Dec 12, 2021
Merged
@costin
Copy link
Member

costin commented Dec 24, 2021

Fwiw, jackson 2.13.1 was released 6 days ago (at the timing of writing this comment):
https://github.com/FasterXML/jackson-core/releases/tag/jackson-core-2.13.1
https://github.com/FasterXML/jackson-core/milestone/61?closed=1

@nik9000
Copy link
Member Author

nik9000 commented Jan 4, 2022

Once we can upgrade jackson and jackson released 2.14 we can fix #63842.

@ywelsch ywelsch mentioned this pull request Jan 4, 2022
Merged
@nik9000
Copy link
Member Author

nik9000 commented Jan 4, 2022

The bug was finally fixed Azure/azure-sdk-for-java#22042 once they release a new version with the fix, I'll upgrade the libraries.

@fcofdez do you know what version we're waiting on? I can't really tell from the linked issues.

@fcofdez
Copy link
Contributor

fcofdez commented Jan 4, 2022

@nik9000 We're waiting for 12.15.0. They released a beta almost a month ago https://github.com/Azure/azure-sdk-for-java/releases/tag/azure-storage-blob_12.15.0-beta.2 I guess the final release should be around the corner.

@nik9000
Copy link
Member Author

nik9000 commented Jan 4, 2022

@nik9000 We're waiting for 12.15.0. They released a beta almost a month ago https://github.com/Azure/azure-sdk-for-java/releases/tag/azure-storage-blob_12.15.0-beta.2 I guess the final release should be around the corner.

Thanks!

ywelsch added a commit that referenced this pull request Jan 5, 2022
@ywelsch
Check older source-only repos are supported (#82213)
bb8578b 
Checks that archive functionality works with source-only repositories.

Relates #80160
astefan pushed a commit to astefan/elasticsearch that referenced this pull request Jan 7, 2022
@ywelsch @astefan
Check older source-only repos are supported (elastic#82213)
9a6a117 
Checks that archive functionality works with source-only repositories.

Relates elastic#80160
astefan pushed a commit to astefan/elasticsearch that referenced this pull request Jan 7, 2022
@ywelsch @astefan
Check older source-only repos are supported (elastic#82213)
2f41736 
Checks that archive functionality works with source-only repositories.

Relates elastic#80160
elasticsearchmachine pushed a commit that referenced this pull request Jan 13, 2022
@weizijun
add test (#81634)
d07d27e 
add a xcontent filter test that will failed, when random to
`filterOnParser`. The test will fixed when jackson upgrade in
#80160
@mark-vieira mark-vieira added v8.2.0 and removed v8.1.0 labels Feb 2, 2022
@weizijun weizijun mentioned this pull request Feb 25, 2022
Merged
@nik9000
Copy link
Member Author

nik9000 commented Mar 9, 2022

Overtaken by #83884

@nik9000 nik9000 closed this Mar 9, 2022
@DaveCTurner DaveCTurner mentioned this pull request Dec 24, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fcofdez fcofdez Awaiting requested review from fcofdez

Assignees
No one assigned
Labels
:Core/Infra/Core Core issues without another label :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue Team:Core/Infra Meta label for core/infra team Team:Distributed (Obsolete) Meta label for distributed team (obsolete). Replaced by Distributed Indexing/Coordination. test-windows Trigger CI checks on Windows
Projects
None yet
Milestone
No milestone
9 participants
@nik9000 @elasticmachine @fcofdez @grcevski @weizijun @ChrisHegarty @costin @mark-vieira @elasticsearchmachine