-
Notifications
You must be signed in to change notification settings - Fork 42
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
If you forget to create a schema, weird error messages ensue #54
Comments
I agree that there's room to make this clear - you're right that the reason we don't have this yet is the lack of a clear way to say "this schema is complete" (so we wait for you to use it), but some issues like multiple I'm open to a PR for that schema check, and I think the ability to define multiple hash keys is a bug. |
I should note we already have the The duplicate definition of hash keys without a clear resolution still concerns me at bug-level, though. |
model_valid? in the documentation. Notably, I should add a docstring for that. |
Well, if you like my idea about a cached check at Let me know if you'd be interested in a PR; I'd be happy to put one together for you. |
or another option; we could do it on the first |
I'm open to a more user friendly error in those cases, yes. I'm inclined to think that duplicate invocation of the |
Rolling this as something to look at when we rev the major version of the library for the modularized SDK. |
Adding a feature request to update model_valid? documentation and raise when a second hash key is declared. |
I am a stupid idiot, and I forgot to add
hash_key: true
to the actual, uh,hash_key
. This is a dumb thing that dumb people do and I lost hours from it due to my own stupidity.However, the library never told me "Hey, idiot, I don't know how to handle your aws-record object because you never told me what your actual keys were." That would've been nice :(
I suspect, also, that you can probably inadvertently specify two
hash_key
's, and the library won't care.Ideally, when the Class is first created might be the best time to check for this, but I think you might not ever get a signal that 'oh, yeah, the user is finished defining their class'.
So instead, maybe we could put something in the
save()
method that, when it's walking through the keys to see if any of them have changed (to see if this is a 'new record' or an 'update'), we can use the following algorithm:false
) is true, if so skip this check. Something likeschema_checked?
Aws::Record::SchemaNotSpecified
orAws::Record::SchemaInvalid
, ideally with some helpful further information like "No Hash Key defined" or "More than one Hash Key defined" or "More than one Range Key Defined".schema_checked?
totrue
, so this code doesn't get run again.Perhaps that optimization isn't necessary, but lots of people who use DynamoDB use it for performance reasons, and so if they're really beating up on the DB, then repeatedly walking through the defined schema seems wasteful.
The text was updated successfully, but these errors were encountered: