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

DM-18438: Add ability to specify field lengths for DatabaseDict and use that #137

Merged
merged 2 commits into from Mar 18, 2019

Conversation

timj
Copy link
Member

@timj timj commented Mar 13, 2019

PosixDatastore now defines the lengths for PosixDatastoreRecords

@timj timj requested a review from natelust March 13, 2019 20:25
PosixDatastore now defines the lengths for PosixDatastoreRecords
@timj
Copy link
Member Author

timj commented Mar 13, 2019

@natelust I decided to pass in a separate dict with the lengths. I could change it so that the length could be passed in through the types directly if you preferred. ie:

        types = {"path": (str, 256), "formatter": (str, 128), "storage_class": (str, 64),
                 "file_size": int, "checksum": (str, 64), "dataset_id": int}

value = namedtuple("TestValue", ["y", "z"])
data = {
0: value(y="zero", z=0.0),
1: value(y="this is too long", z=0.1),
Copy link
Contributor

Choose a reason for hiding this comment

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

My only comment is that it might be nice to name these keys "passing" and "failing" or something like that rather than generic 0 and 1 magic numbers, but its in test code, and I dont feel very strongly about it.

sqlite ignores varchar length. An explicit check constraint
is required to force a length check.  This commit adds that
and a corresponding test to ensure that the lengths have
been applied properly.
@timj timj merged commit bca4981 into master Mar 18, 2019
@timj timj deleted the tickets/DM-18438 branch March 18, 2019 23:24
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.

None yet

2 participants