Page MenuHomeSoftware Heritage
Differential D7233

Replace xmltodict with ElementTree to handle SWORD documents
ClosedPublic
Actions

Authored by vlorentz on Feb 23 2022, 7:03 PM.

Details

Reviewers
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T3971: swh.deposit.errors.ParserError: out of memory: line 1, column 0
Commits
rDDEPcf92b5f2feb6: Replace xmltodict with ElementTree to handle SWORD documents
Summary

xmltodict was already on the way out for the deposit, and the latest
libexpat security update broke it entirely when dealing with namespaces,
which means we cannot use it until this is addressed.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1006317

Functional changes of this commit:

  1. No more writes to the 'metadata' jsonb column in the DB (as it strongly depends on xmltodict)
  2. ServiceDocumentDepositClient always outputs a list of collections, instead of None/dict/List[dict] depending on the number of collections (artefact of using xmltodict, which is replaced by proper parsing)

Diff Detail

Repository
rDDEP Push deposit
Branch
drop-xmltodict
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27089
Build 42366: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 42365: arc lint + arc unit

Event Timeline

vlorentz created this revision.Feb 23 2022, 7:03 PM
Herald added a reviewer: Reviewers. · View Herald TranscriptFeb 23 2022, 7:03 PM
swh-public-ci added a comment.Feb 23 2022, 7:11 PM

Build has FAILED

Patch application report for D7233 (id=26214)

Could not rebase; Attempt merge onto 5a7d59cb2c...

Merge made by the 'recursive' strategy.
 swh/deposit/api/common.py                          |  39 ++----
 swh/deposit/api/edit.py                            |   7 +-
 swh/deposit/api/private/deposit_read.py            |   2 -
 swh/deposit/cli/client.py                          |   8 +-
 swh/deposit/client.py                              |  80 +++++++++---
 swh/deposit/config.py                              |   1 -
 swh/deposit/parsers.py                             |  10 +-
 swh/deposit/tests/api/conftest.py                  |   3 +-
 .../tests/api/test_collection_add_to_origin.py     |   7 +-
 swh/deposit/tests/api/test_collection_list.py      |  47 ++++---
 swh/deposit/tests/api/test_collection_post_atom.py |  71 +++++-----
 .../tests/api/test_collection_post_binary.py       |  52 +++++---
 .../tests/api/test_collection_post_multipart.py    |  39 +++---
 .../tests/api/test_collection_reuse_slug.py        |  31 +++--
 swh/deposit/tests/api/test_delete.py               |   8 +-
 .../tests/api/test_deposit_private_check.py        |   7 +-
 .../api/test_deposit_private_read_metadata.py      |   7 -
 swh/deposit/tests/api/test_deposit_schedule.py     |  20 ++-
 swh/deposit/tests/api/test_deposit_state.py        |  64 ++++++---
 swh/deposit/tests/api/test_deposit_update_atom.py  |  64 ++++-----
 .../tests/api/test_deposit_update_binary.py        |  19 +--
 swh/deposit/tests/api/test_get_file.py             |  22 ++--
 swh/deposit/tests/api/test_parsers.py              |  97 +++++---------
 swh/deposit/tests/cli/test_client.py               | 144 +++++++++++++++------
 swh/deposit/tests/conftest.py                      |  14 +-
 swh/deposit/tests/test_client_module.py            |   2 +-
 swh/deposit/utils.py                               |  14 --
 27 files changed, 490 insertions(+), 389 deletions(-)
Changes applied before test
commit 443015cc03b5254a539bcd97b96b6a79e426585c
Merge: 5a7d59cb 486eb888
Author: Jenkins user <jenkins@localhost>
Date:   Wed Feb 23 18:03:15 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 486eb888d8f63d28dbeb3a0f549ea37594828bec
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 19:00:09 2022 +0100

    Replace xmltodict with ElementTree to handle SWORD documents
    
    xmltodict was already on the way out for the deposit, and the latest
    libexpat security update broke it entirely when dealing with namespaces,
    which means we cannot use it until this is addressed.
    
    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1006317
    
    Functional changes of this commit:
    
    1. No more writes to the 'metadata' jsonb column in the DB (as it
       strongly depends on xmltodict)
    2. ServiceDocumentDepositClient always outputs a list of collections,
       instead of None/dict/List[dict] depending on the number of
       collections (artefact of using xmltodict, which is replaced by proper
       parsing)

commit 68eb441cb90edbab5b1fadb1b411c106a6c9c70e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 13:44:07 2022 +0100

    Remove metadata_dict from the API
    
    No one uses that, and it's redundant, as we provide the original XML

commit 189ba2f7cc9c5b35891b23adf19872b8f230b7e8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 13:43:33 2022 +0100

    Remove unnecessary use of BytesIO

Link to build: https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/744/
See console output for more information: https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/744/console

Harbormaster returned this revision to the author for changes because remote builds failed.Feb 23 2022, 7:11 PM
Harbormaster failed remote builds in B27087: Diff 26214!
vlorentz updated this revision to Diff 26215.Feb 23 2022, 7:16 PM

fix cli

vlorentz updated this revision to Diff 26216.Feb 23 2022, 7:22 PM

wait, that was nonsense

swh-public-ci added a comment.Feb 23 2022, 7:24 PM

Build is green

Patch application report for D7233 (id=26215)

Could not rebase; Attempt merge onto 5a7d59cb2c...

Merge made by the 'recursive' strategy.
 swh/deposit/api/common.py                          |  39 ++----
 swh/deposit/api/edit.py                            |   7 +-
 swh/deposit/api/private/deposit_read.py            |   2 -
 swh/deposit/cli/client.py                          |   2 +-
 swh/deposit/client.py                              |  80 +++++++++---
 swh/deposit/config.py                              |   1 -
 swh/deposit/parsers.py                             |  10 +-
 swh/deposit/tests/api/conftest.py                  |   3 +-
 .../tests/api/test_collection_add_to_origin.py     |   7 +-
 swh/deposit/tests/api/test_collection_list.py      |  47 ++++---
 swh/deposit/tests/api/test_collection_post_atom.py |  71 +++++-----
 .../tests/api/test_collection_post_binary.py       |  52 +++++---
 .../tests/api/test_collection_post_multipart.py    |  39 +++---
 .../tests/api/test_collection_reuse_slug.py        |  31 +++--
 swh/deposit/tests/api/test_delete.py               |   8 +-
 .../tests/api/test_deposit_private_check.py        |   7 +-
 .../api/test_deposit_private_read_metadata.py      |   7 -
 swh/deposit/tests/api/test_deposit_schedule.py     |  20 ++-
 swh/deposit/tests/api/test_deposit_state.py        |  64 ++++++---
 swh/deposit/tests/api/test_deposit_update_atom.py  |  64 ++++-----
 .../tests/api/test_deposit_update_binary.py        |  19 +--
 swh/deposit/tests/api/test_get_file.py             |  22 ++--
 swh/deposit/tests/api/test_parsers.py              |  97 +++++---------
 swh/deposit/tests/cli/test_client.py               | 144 +++++++++++++++------
 swh/deposit/tests/conftest.py                      |  14 +-
 swh/deposit/tests/test_client_module.py            |   2 +-
 swh/deposit/utils.py                               |  14 --
 27 files changed, 487 insertions(+), 386 deletions(-)
Changes applied before test
commit abe23026ddccdc59660fc24c25266158a51fbcb6
Merge: 5a7d59cb d3704492
Author: Jenkins user <jenkins@localhost>
Date:   Wed Feb 23 18:16:45 2022 +0000

    Merge branch 'diff-target' into HEAD

commit d370449226af418750903c808a409948a2a4fb71
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 19:00:09 2022 +0100

    Replace xmltodict with ElementTree to handle SWORD documents
    
    xmltodict was already on the way out for the deposit, and the latest
    libexpat security update broke it entirely when dealing with namespaces,
    which means we cannot use it until this is addressed.
    
    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1006317
    
    Functional changes of this commit:
    
    1. No more writes to the 'metadata' jsonb column in the DB (as it
       strongly depends on xmltodict)
    2. ServiceDocumentDepositClient always outputs a list of collections,
       instead of None/dict/List[dict] depending on the number of
       collections (artefact of using xmltodict, which is replaced by proper
       parsing)

commit 68eb441cb90edbab5b1fadb1b411c106a6c9c70e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 13:44:07 2022 +0100

    Remove metadata_dict from the API
    
    No one uses that, and it's redundant, as we provide the original XML

commit 189ba2f7cc9c5b35891b23adf19872b8f230b7e8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 13:43:33 2022 +0100

    Remove unnecessary use of BytesIO

See https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/745/ for more details.

Harbormaster completed remote builds in B27088: Diff 26215.Feb 23 2022, 7:24 PM
swh-public-ci added a comment.Feb 23 2022, 7:30 PM

Build is green

Patch application report for D7233 (id=26216)

Rebasing onto 5a7d59cb2c...

First, rewinding head to replay your work on top of it...
Applying: Remove unnecessary use of BytesIO
Applying: Remove metadata_dict from the API
Applying: Replace xmltodict with ElementTree to handle SWORD documents
Changes applied before test
commit 11bbdc6e26ef61e43f4ac58997e7b770706eee03
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 19:00:09 2022 +0100

    Replace xmltodict with ElementTree to handle SWORD documents
    
    xmltodict was already on the way out for the deposit, and the latest
    libexpat security update broke it entirely when dealing with namespaces,
    which means we cannot use it until this is addressed.
    
    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1006317
    
    Functional changes of this commit:
    
    1. No more writes to the 'metadata' jsonb column in the DB (as it
       strongly depends on xmltodict)
    2. ServiceDocumentDepositClient always outputs a list of collections,
       instead of None/dict/List[dict] depending on the number of
       collections (artefact of using xmltodict, which is replaced by proper
       parsing)

commit 16ee56352bac1bae78620034800dce9427d4cf3b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 13:44:07 2022 +0100

    Remove metadata_dict from the API
    
    No one uses that, and it's redundant, as we provide the original XML

commit 149317dc3c1ecbaaa795547bd5331e0bddfab2f8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 13:43:33 2022 +0100

    Remove unnecessary use of BytesIO

See https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/746/ for more details.

Harbormaster completed remote builds in B27089: Diff 26216.Feb 23 2022, 7:30 PM
vlorentz requested review of this revision.Feb 23 2022, 7:30 PM
ardumont accepted this revision.Feb 23 2022, 8:19 PM
ardumont added a subscriber: ardumont.

lgtm

swh/deposit/client.py
437

i'm gonna believe you on that one ;)

swh/deposit/tests/api/test_deposit_update_atom.py
581

clearer ;)

This revision is now accepted and ready to land.Feb 23 2022, 8:19 PM
ardumont added a comment.Feb 24 2022, 9:44 AM

This needs a rebase (it's missing the last commit from master but nothing is to be modified after it).

Can you please rebase, update and push so that i can rebase my last [1] 2 deposit diffs [2] on it?

[1] I should be done for the deposit modifications

[2] D7227 D7228

ardumont added a child revision: D7227: cli: Warn when metadata-only deposit without metadata provenance.Feb 24 2022, 9:48 AM
vlorentz updated this revision to Diff 26222.Feb 24 2022, 9:50 AM

rebase

swh-public-ci added a comment.Feb 24 2022, 10:00 AM

Build is green

Patch application report for D7233 (id=26222)

Could not rebase; Attempt merge onto 5a7d59cb2c...

Updating 5a7d59cb..cf92b5f2
Fast-forward
 swh/deposit/api/common.py                          |  39 ++----
 swh/deposit/api/edit.py                            |   7 +-
 swh/deposit/api/private/deposit_read.py            |   2 -
 swh/deposit/cli/client.py                          |   2 +-
 swh/deposit/client.py                              |  78 ++++++++---
 swh/deposit/config.py                              |   1 -
 swh/deposit/parsers.py                             |  10 +-
 swh/deposit/tests/api/conftest.py                  |   3 +-
 .../tests/api/test_collection_add_to_origin.py     |   7 +-
 swh/deposit/tests/api/test_collection_list.py      |  47 ++++---
 swh/deposit/tests/api/test_collection_post_atom.py |  71 +++++-----
 .../tests/api/test_collection_post_binary.py       |  52 +++++---
 .../tests/api/test_collection_post_multipart.py    |  39 +++---
 .../tests/api/test_collection_reuse_slug.py        |  31 +++--
 swh/deposit/tests/api/test_delete.py               |   8 +-
 .../tests/api/test_deposit_private_check.py        |   7 +-
 .../api/test_deposit_private_read_metadata.py      |   7 -
 swh/deposit/tests/api/test_deposit_schedule.py     |  20 ++-
 swh/deposit/tests/api/test_deposit_state.py        |  64 ++++++---
 swh/deposit/tests/api/test_deposit_update_atom.py  |  64 ++++-----
 .../tests/api/test_deposit_update_binary.py        |  19 +--
 swh/deposit/tests/api/test_get_file.py             |  22 ++--
 swh/deposit/tests/api/test_parsers.py              |  97 +++++---------
 swh/deposit/tests/cli/test_client.py               | 144 +++++++++++++++------
 swh/deposit/tests/conftest.py                      |  14 +-
 swh/deposit/tests/test_client_module.py            |   2 +-
 swh/deposit/utils.py                               |  14 --
 27 files changed, 485 insertions(+), 386 deletions(-)
Changes applied before test
commit cf92b5f2feb6e9ca3e071e98922417fd530761c3
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 19:00:09 2022 +0100

    Replace xmltodict with ElementTree to handle SWORD documents
    
    xmltodict was already on the way out for the deposit, and the latest
    libexpat security update broke it entirely when dealing with namespaces,
    which means we cannot use it until this is addressed.
    
    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1006317
    
    Functional changes of this commit:
    
    1. No more writes to the 'metadata' jsonb column in the DB (as it
       strongly depends on xmltodict)
    2. ServiceDocumentDepositClient always outputs a list of collections,
       instead of None/dict/List[dict] depending on the number of
       collections (artefact of using xmltodict, which is replaced by proper
       parsing)

commit 5998e19648f4a987dc024935f20d7e0adae31dc8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 13:44:07 2022 +0100

    Remove metadata_dict from the API
    
    No one uses that, and it's redundant, as we provide the original XML

commit 79be8e1c74e15607d4ced35fc2f2de1359c02ecb
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Feb 23 13:43:33 2022 +0100

    Remove unnecessary use of BytesIO

See https://jenkins.softwareheritage.org/job/DDEP/job/tests-on-diff/750/ for more details.

Harbormaster completed remote builds in B27095: Diff 26222.Feb 24 2022, 10:00 AM
Closed by commit rDDEPcf92b5f2feb6: Replace xmltodict with ElementTree to handle SWORD documents (authored by vlorentz). · Explain WhyFeb 24 2022, 10:02 AM
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDDEPcf92b5f2feb6: Replace xmltodict with ElementTree to handle SWORD documents.
vlorentz added a task: T3971: swh.deposit.errors.ParserError: out of memory: line 1, column 0.Feb 24 2022, 10:05 AM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
deposit/
api/
39 lines95%
Loading...
7 lines100%
Loading...
private/
2 lines97%
Loading...
cli/
2 lines97%
Loading...
78 lines95%
Loading...
1 line94%
Loading...
10 lines100%
Loading...
tests/
api/
3 lines100%
Loading...
7 lines100%
Loading...
47 lines100%
Loading...
71 lines100%
Loading...
52 lines100%
Loading...
39 lines100%
Loading...
31 lines100%
Loading...
8 lines100%
Loading...
7 lines100%
Loading...
7 lines100%
Loading...
20 lines100%
Loading...
64 lines98%
Loading...
64 lines100%
Loading...
19 lines100%
Loading...
22 lines100%
Loading...
97 lines100%
Loading...
cli/
144 lines99%
Loading...
14 lines97%
Loading...
2 lines100%
Loading...
14 lines95%
Loading...

Diff 26216

View Options

swh/deposit/api/common.py

Loading...
View Options

swh/deposit/api/edit.py

Loading...
View Options

swh/deposit/api/private/deposit_read.py

Loading...
View Options

swh/deposit/cli/client.py

Loading...
View Options

swh/deposit/client.py

Loading...
View Options

swh/deposit/config.py

Loading...
View Options

swh/deposit/parsers.py

Loading...
View Options

swh/deposit/tests/api/conftest.py

Loading...
View Options

swh/deposit/tests/api/test_collection_add_to_origin.py

Loading...
View Options

swh/deposit/tests/api/test_collection_list.py

Loading...
View Options

swh/deposit/tests/api/test_collection_post_atom.py

Loading...
View Options

swh/deposit/tests/api/test_collection_post_binary.py

Loading...
View Options

swh/deposit/tests/api/test_collection_post_multipart.py

Loading...
View Options

swh/deposit/tests/api/test_collection_reuse_slug.py

Loading...
View Options

swh/deposit/tests/api/test_delete.py

Loading...
View Options

swh/deposit/tests/api/test_deposit_private_check.py

Loading...
View Options

swh/deposit/tests/api/test_deposit_private_read_metadata.py

Loading...
View Options

swh/deposit/tests/api/test_deposit_schedule.py

Loading...
View Options

swh/deposit/tests/api/test_deposit_state.py

Loading...
View Options

swh/deposit/tests/api/test_deposit_update_atom.py

Loading...
View Options

swh/deposit/tests/api/test_deposit_update_binary.py

Loading...
View Options

swh/deposit/tests/api/test_get_file.py

Loading...
View Options

swh/deposit/tests/api/test_parsers.py

Loading...
View Options

swh/deposit/tests/cli/test_client.py

Loading...
View Options

swh/deposit/tests/conftest.py

Loading...
View Options

swh/deposit/tests/test_client_module.py

Loading...
View Options

swh/deposit/utils.py

Loading...
About · Developer information · Legal