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

Add row count check to spot check data #575

Merged
merged 1 commit into from
Dec 13, 2022
Merged

Add row count check to spot check data #575

merged 1 commit into from
Dec 13, 2022

Conversation

jordanpadams
Copy link
Member

Make sure we are not trying to read a record beyond the defined row count for the product file.

Resolves #554

⚙️ Test Data and/or Report

https://github.com/NASA-PDS/validate/actions/runs/3670637715

Make sure we are not trying to read a record beyond the defined row count for the product file.

Resolves #554
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.

👍

@nutjob4life nutjob4life merged commit 05f8d92 into main Dec 13, 2022
@nutjob4life nutjob4life deleted the i554 branch December 13, 2022 01:01
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.

There are two problems that persist with this fix.

  1. this.currentTableReader is instantiated once but this.currentTableReader.readNext() is called from many functions in this class. Therefore you cannot know where readNext() is pointing without a reset back to the beginning.
  2. If next the reader is giving valid record (not null) past end of table, then that is really an error in the reader.

I guess me be slow allowed this symptomatic fix to be merged.

@jordanpadams
Copy link
Member Author

@al-niessner great points.

  1. this.currentTableReader is instantiated once but this.currentTableReader.readNext() is called from many functions in this class. Therefore you cannot know where readNext() is pointing without a reset back to the beginning.

agreed. this is really a problem with the overall architecture of the code. I am pretty sure if you roll up where those readNext() calls are happening, only one of those can be executed depending upon the type of product being validated. more clear encapsulation would probably help here.

  1. If next the reader is giving valid record (not null) past end of table, then that is really an error in the reader.

the reader was throwing an IOException because we were trying to read too far. that being said, we actually have a feature request to actually try to "peak" past the defined table to see if the definition is invalid (e.g. label defines table with 5 records, but the products actually has 10 records)

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.

--spot-check-data flag throws IOException
3 participants