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

Update open/close of object reader #566

Merged
merged 1 commit into from
Dec 2, 2022
Merged

Update open/close of object reader #566

merged 1 commit into from
Dec 2, 2022

Conversation

jordanpadams
Copy link
Member

Previous functionality opened and closed the reader within a recursive method. Moved thie open/close up a level in execution.

Resolves #564

⚙️ Test Data and/or Report

$ validate-3.1.0-SNAPSHOT/bin/validate -t ~/test/geo_issue_20221201/s_07171001_sim.xml

PDS Validate Tool Report

Configuration:
   Version                       3.1.0-SNAPSHOT
   Date                          2022-12-01T17:26:25Z

Parameters:
   Targets                       [file:/Users/jpadams/test/geo_issue_20221201/s_07171001_sim.xml]
   Severity Level                WARNING
   Recurse Directories           true
   File Filters Used             [*.xml, *.XML]
   Data Content Validation       on
   Product Level Validation      on
   Max Errors                    100000
   Registered Contexts File      /Users/jpadams/proj/pds/pdsen/workspace/validate/validate-3.1.0-SNAPSHOT/resources/registered_context_products.json



Product Level Validation Results

  PASS: file:/Users/jpadams/test/geo_issue_20221201/s_07171001_sim.xml
        1 product validation(s) completed

Summary:

  0 error(s)
  0 warning(s)

  Product Validation Summary:
    1          product(s) passed
    0          product(s) failed
    0          product(s) skipped

  Referential Integrity Check Summary:
    0          check(s) passed
    0          check(s) failed
    0          check(s) skipped


End of Report
Completed execution in 9730 ms

Test data emailed to reviewers via LFT

Previous functionality opened and closed the reader within a recursive method. Moved thie open/close up a level in execution.

Resolves #564
Copy link
Contributor

@al-niessner al-niessner left a comment

Choose a reason for hiding this comment

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

ArrayObject.open() is not thread safe. Going to really bite when validate becomes threaded to run faster.

Asking the user to look at data type vs size is not going to go over well in the end. The checks in ArrayObject that check dimensions and index can return different (less generic) exceptions to produce a meaningful message appropriate the actual failure. If the reference to data type is data type size vs number of total bytes available then that should be simple to check as well given actual type is returned by ArrayObject.

Copy link
Member

@nutjob4life nutjob4life left a comment

Choose a reason for hiding this comment

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

👍

@jordanpadams
Copy link
Member Author

ArrayObject.open() is not thread safe. Going to really bite when validate becomes threaded to run faster.

@al-niessner we (more specifically @galenatjpl) actually tried to make Validate multi-threaded a few years ago but ran into some issues with the XML libraries not being thread safe

Asking the user to look at data type vs size is not going to go over well in the end. The checks in ArrayObject that check dimensions and index can return different (less generic) exceptions to produce a meaningful message appropriate the actual failure. If the reference to data type is data type size vs number of total bytes available then that should be simple to check as well given actual type is returned by ArrayObject.

@al-niessner totally agree. will create a separate ticket to refactor ArrayObject to provide better context to the user.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Array object validation regression in v3.0.3
3 participants