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

Recognize pre-release tables #156

Open
wants to merge 1 commit into
base: engineering
Choose a base branch
from
Open

Recognize pre-release tables #156

wants to merge 1 commit into from

Conversation

durbrow
Copy link
Collaborator

@durbrow durbrow commented Oct 4, 2024

This code is not wrong. As far as KDB is concerned, it is correct. It is the same logic that KDBPathType uses, and KDBPathContents should match KDBPathType. But as far as I can tell, we have no files that are so old that KDBPathType would recognize them as pre-release tables.

@durbrow durbrow requested a review from aboshkin October 4, 2024 20:45
Copy link
Contributor

@aboshkin aboshkin left a comment

Choose a reason for hiding this comment

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

Lokks good to me. I understand this is not easy to test, but since you mentioned SRAO-316, maybe add an sra-info test on one of those old runs?

@durbrow
Copy link
Collaborator Author

durbrow commented Oct 5, 2024

The problem with the ones from the SRAO ticket is that KDB does not recognize them as pre-release tables. I thought they'd work for a test case but I was wrong. That's why I don't have a test case. :(

You can pass it or fail it. It is not testable (and neither is KDBPathType in this case). Maybe it's time to deprecate the concept of a pre-release KDB table.

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.

2 participants