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

Support for aws-sdk 2 #4

Merged
merged 46 commits into from Jul 7, 2015
Merged

Support for aws-sdk 2 #4

merged 46 commits into from Jul 7, 2015

Conversation

peeyush1234
Copy link
Contributor

I would like to get some feedback on the pull request for using aws-sdk 2. Once this pull request is accepted, the dynamoid major version should be bumped as the change is not compatible with aws-sdk 1.

In the meantime (till, I get the feedback on the pull request), I will update the README section to reflect the changes.

@peeyush1234
Copy link
Contributor Author

@BilalBudhani, any feedback will be appreciated.

@BilalBudhani
Copy link

I've tested this in my current project. Everything seems to be working fine at the moment. I just ran into one breaking issue which is when the field is not defined in the model but it exits in DynamoDB item then it raises a error resulting in breaking the code. This behaviour should be handled as there are chances of DynamoDB fields being out of sync with the model. It might break some legacy applications.

@philipmw
Copy link
Member

@BilalBudhani, how do you suggest to handle that situation? One problem I see is that without a field declaration, Dynamoid won't know how to materialize the string value.

@BilalBudhani
Copy link

@philipmw Can we just simply ignore the fields which is not declared in the model? This way Dynamoid will only provide access to fields which has been declared. Make sense?

@philipmw
Copy link
Member

@BilalBudhani, I must have misunderstood your original bug report. I agree that Dynamoid should ignore fields that aren't declared on the model. It sounds like you're saying that if a DynamoDB record has attributes that aren't declared on the Ruby model, Dynamoid raises an exception when materializing the model instance. Is that right?

@BilalBudhani
Copy link

@philipmw Yes. When DynamoDB record has a field which is not declared on Dynamoid model. It raises an exception.

@philipmw
Copy link
Member

@BilalBudhani would you open an issue about this? I assume this behavior exists on the master branch too.

@BilalBudhani
Copy link

@philipmw Sure. I will open an issue for it.

@loganb
Copy link
Contributor

loganb commented Jun 30, 2015

@philipmw, can you move your commits that remove functionality and/or fix issues other than AWS-SDK-2 to a separate PR?

BTW. You can do this easily by pushing a new branch with them, then resetting the aws-sdk-2 branch back a few commits.

@philipmw
Copy link
Member

philipmw commented Jul 6, 2015

@loganb, done.

loganb added a commit that referenced this pull request Jul 7, 2015
@loganb loganb merged commit be34f57 into master Jul 7, 2015
@philipmw philipmw deleted the clientv2 branch July 7, 2015 21:49
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 1c8eb57 on clientv2 into ** on master**.

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

5 participants