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

Fix typo (Dynanmo->Dynamo). #218

Merged
merged 2 commits into from
Aug 24, 2015
Merged

Conversation

pkulev
Copy link
Contributor

@pkulev pkulev commented Aug 20, 2015

  • Fixed typo in DynamoDB exception class name.
  • Fixed tests.
  • Provided alias for backward compatibility.

@jamesls
Copy link
Member

jamesls commented Aug 20, 2015

We should definitely get this fixed. Only question is whether it's worth aliasing the typo'd exception for backwards compatibility. I could go either way on that second point.

@kyleknap - thoughts?

@rayluo
Copy link
Contributor

rayluo commented Aug 22, 2015

For that second point, the fancy way is to provide aliases for backwards compatibility, AND also mention they are deprecated in the release note, and then a couple versions later we will get rid of them.

That being said, in this case the previous behavior can be considered as a bug, and a bug is always fixed without "backward compatibility".

@rayluo rayluo added the bug This issue is a confirmed bug. label Aug 22, 2015
@pkulev
Copy link
Contributor Author

pkulev commented Aug 24, 2015

Ok, guys, I added alias.
But IMHO this is not such case when there's a need to provide aliases.
This is bug, not sort of deprecated thing.

@jamesls
Copy link
Member

jamesls commented Aug 24, 2015

:shipit: Looks good to me.

@kyleknap
Copy link
Contributor

@jamesls
The error is there for validation purposes. It tells you that the condition you tried to use cannot be used. I am having trouble imagining someone trying to catch this exception specifically because you really cannot do anything with the error or have logic that relies on that error being thrown because you have to physically fix your condition statement in order to even be able to make a successful call.

However, I suppose if you are building an application on top of the dynamodb library, and you wanted to modify the message sent back for these validations to the user that would break the application. As much as I really do not like that spelling mistake in there it is the right thing to alias it for backwards compatibility and deprecate it (since it is a bug/mistake) as @rayluo mentioned, and which I noticed @pankshok you already made.

Thanks for the catch. Merging.

kyleknap added a commit that referenced this pull request Aug 24, 2015
@kyleknap kyleknap merged commit 21350fc into boto:develop Aug 24, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a confirmed bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants