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

Add collection element types #307

Merged
merged 8 commits into from Oct 20, 2018
Merged

Conversation

andrykonchin
Copy link
Member

Updates:

  • extended supported types list of :of option in field declaration for set field
  • add :of option for array field

of option now supports all scalar types (except boolean) like integer, number, date, datetime, serializable and custom types.

Example:

field :values, :set, of: :datetime

Some type declarations have options like store_as_string or serializer. It's possible to specify them for elements type:

field :values, :array, of: { serialized: { serializer: JSON } }

Related Issue #272

Copy link
Member

@pboling pboling left a comment

Choose a reason for hiding this comment

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

Left some comments on documentation. Haven't reviewed the rest yet.

README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated
@@ -202,6 +206,31 @@ class Document
end
```

Some type declaration have options like `store_as_string` for `datetime`
or `serializer` for `serializable`. It's possible to specify them for elements type:
Copy link
Member

Choose a reason for hiding this comment

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

I do not understand this statement:

It's possible to specify them for elements type:

Does it mean this,

It's possible to specify a type declaration for each field:

Copy link
Member Author

Choose a reason for hiding this comment

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

@pboling Hm, no.

I meant user can declare usual field options when declares set elements type e.g. set of datetime and store datetime in numeric or string format.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure how to improve it then. I still don't fully understand. Maybe add a few more fields to complete the example?

README.md Outdated
```

As far as DynamoDB doesn't allow storing empty strings in Set
when `Dynamoid` saves a document it removes all empty strings in `set` fields.
Copy link
Member

Choose a reason for hiding this comment

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

Improving grammar for clarity:

DynamoDB doesn't allow empty strings in fields configured as set.
Abiding by this restriction, when Dynamoid saves a document it removes all empty strings in set fields.

README.md Outdated

#### Note on array type

`array` field declaration supports `of` option as well:
Copy link
Member

Choose a reason for hiding this comment

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

Is there any guidance we can add relative to why or when one should use a set and when one should use an array, aside from the element type restrictions? This may not be the right place for that sort of thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

@pboling

The guidance could be found in DynamoDB's documentation as far as Dynamoid's types are mapped straightforward to DynamoDB's - array on List and set on Set.

Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell, the mapping you refer to is not in the README anywhere. Might be in some wiki, but it would be helpful to clarify in the README. Most Rubyists are not familiar with Ruby's Set class:

Amazon DynamoBD Dynamoid gem
List Array
Set Set

@andrykonchin
Copy link
Member Author

andrykonchin commented Oct 15, 2018

@pboling Updated Readme. Please check new wording.

@andrykonchin
Copy link
Member Author

@pboling ping

Did you have a chance to look at new wording here? Have I missed anything?

@pboling
Copy link
Member

pboling commented Oct 20, 2018

@andrykonchin The latest documentation looks great!

@andrykonchin andrykonchin merged commit 24b9ac7 into master Oct 20, 2018
@andrykonchin andrykonchin deleted the add-collection-element-types branch October 20, 2018 15:43
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