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

Bumping up the version of jackson to 2.12.0 #67214

Closed
wants to merge 7 commits into from

Conversation

jamesrowe08
Copy link

@jamesrowe08 jamesrowe08 commented Jan 8, 2021

This will close #66555

API calls mentioned in that bug now work correctly:

curl -u elastic:password -H 'Content-Type: application/yaml' -X GET "localhost:9200/_ingest/pipeline/yaml_test"

yaml_test:
  description: "YAML unmarshal bug demo"
  processors:
  - script:
      lang: "painless"
      params:
        number_base10: 1
        number_base16: 1
        string: "hello"
      source: "params.forEach((k, v) -> ctx[k] = v);"
curl -u elastic:password -X GET "localhost:9200/_ingest/pipeline/yaml_test?pretty"
"yaml_test" : {
    "description" : "YAML unmarshal bug demo",
    "processors" : [
      {
        "script" : {
          "lang" : "painless",
          "params" : {
            "number_base10" : 1,
            "number_base16" : 1,
            "string" : "hello"
          },
          "source" : "params.forEach((k, v) -> ctx[k] = v);"
        }
      }
    ]
  }
}

@rjernst rjernst added the :Core/Infra/Core Core issues without another label label Jan 15, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Jan 15, 2021
@elasticmachine
Copy link
Collaborator

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

@rjernst
Copy link
Member

rjernst commented Jan 15, 2021

I believe snakeyaml needs to be bumped as well. It looks like jackson-dataformat-yaml 2.12.0 uses snakeyaml with version 1.27.

@jamesrowe08
Copy link
Author

This change has been made @rjernst

@rjernst
Copy link
Member

rjernst commented Jan 19, 2021

@elasticmachine ok to test

@rjernst rjernst self-assigned this Jan 19, 2021
@rjernst
Copy link
Member

rjernst commented Jan 19, 2021

@jamesrowe08 I've pushed an update to the SHAs that needs to go along with this version bump. Note, however, that it is generally inadvisable to use the master branch as the source of a pull request. I recommend for any future pull requests you create a branch off of master.

@rjernst
Copy link
Member

rjernst commented Jan 27, 2021

@elasticmachine update branch

@rjernst
Copy link
Member

rjernst commented Jan 28, 2021

@elasticmachine update branch

@mark-vieira
Copy link
Contributor

@elasticmachine update branch

1 similar comment
@fcofdez
Copy link
Contributor

fcofdez commented Feb 5, 2021

@elasticmachine update branch

@fcofdez
Copy link
Contributor

fcofdez commented Feb 8, 2021

There's recent change in Jackson (FasterXML/jackson-dataformat-xml#411) that changed a default behaviour that would prevent upgrading to this version, as it produces an endless loop in the azure sdk code. See
Azure/azure-sdk-for-java#9465 (comment) for more context.

@rjernst
Copy link
Member

rjernst commented Feb 8, 2021

Thanks for tracking that down @fcofdez. I don't think we can upgrade yet to Jackson 2.12.0 because of this. The Azure SDK still depends on 2.11.3. Unfortunately because Jackson is still a dependency of the Elasticsearch server, any use of it in plugins must match the version provided (otherwise we would get jar hell). While it is a goal to continue to not force these dependencies to match, it likely won't happen before this is simply fixed upstream. In the meantime I am marking this PR as stalled.

@rjernst rjernst added the stalled label Feb 8, 2021
@joshbressers
Copy link

@rjernst If I'm correctly reading the tangle of bugs in that azure issue, I think this is fixed now in the current version 12.11.0
Azure/azure-sdk-for-java#18881 (comment)

rjernst added a commit to rjernst/elasticsearch that referenced this pull request May 6, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes elastic#66555
closes elastic#67214
@rjernst
Copy link
Member

rjernst commented May 6, 2021

Thanks @joshbressers. I opened #72833. That requires upgrading Jackson as well in the same commit, so this PR becomes unnecessary. Thanks @jamesrowe08

@rjernst rjernst closed this May 6, 2021
rjernst added a commit that referenced this pull request May 7, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes #66555
closes #67214
rjernst added a commit to rjernst/elasticsearch that referenced this pull request May 12, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes elastic#66555
closes elastic#67214
rjernst added a commit that referenced this pull request May 27, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes #66555
closes #67214

Co-authored-by: Francisco Fernández Castaño <[email protected]>
rjernst added a commit to rjernst/elasticsearch that referenced this pull request May 27, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes elastic#66555
closes elastic#67214

Co-authored-by: Francisco Fernández Castaño <[email protected]>
rjernst added a commit that referenced this pull request May 27, 2021
This commit upgrades the Azure SDK to 12.11.0 and Jackson to 2.12.2. The
Jackson upgrade must happen at the same time due to Azure depending on
this new version of Jackson.

closes #66555
closes #67214

backport #72995
backport #73011

Co-authored-by: Francisco Fernández Castaño <[email protected]>
Co-authored-by: Mark Vieira <[email protected]>
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request 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 elastic#66555
Closes elastic#67214
Closes elastic#80142
nik9000 added a commit to nik9000/elasticsearch that referenced this pull request 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 elastic#66555
Closes elastic#67214
Closes elastic#80142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label stalled Team:Core/Infra Meta label for core/infra team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hex numbers in YAML requests are treated as strings
6 participants