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

Models #18

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Models #18

wants to merge 9 commits into from

Conversation

davidjairala
Copy link
Contributor

No description provided.

@davidjairala
Copy link
Contributor Author

TODO: update readme.

def initialize(attributes = {})
attributes = attributes.symbolize_keys
self.id = attributes.delete(:id)
self.attributes = ((column_defaults || {}).merge(attributes)).symbolize_keys

Choose a reason for hiding this comment

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

do you need to symbolize_keys on column_defaults?

@jrichardlai
Copy link

Looks good I like it :)

module Waistband
class Model

extend ::ActiveModel::Callbacks
Copy link

Choose a reason for hiding this comment

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

I think you might be able to just use ActiveModel::Model but it's been a while since I've used it.

@evantahler
Copy link
Contributor

Philosophy Ramblings:

This is awesome. However, if you you able to save a new model and re-load it quickly, we know that it's very likely that ES will not yet have indexed the new object, and things like model.reload! or just finding the object again be likely to fail. This isn't anything different than how the tool worked before, but the ActiveRecord semantics (because folks are used to working with mySQL et al) might imply that you can think about model.save in the same way.

Since you are working with only one record here (as opposed to multi writes which you might use some of the other waistband sections for), you have the opportunity to enforce that when you call model.save, the next line really will have it available:

  • You can ask the index for its refresh frequency and sleep that long after a save?
  • You can do a while unreadable loop (with a 5s timeout) to try and read the object before moving on from the save method
  • You can cache the object in some way?

Eh, these are all probably terrible ideas... never mind!

@davidjairala
Copy link
Contributor Author

Well, reload! would work using the find method, which is not a search, so
it would not be affected by refresh shenanigans since when you have the id
it knows which node to grab it from.

On Sun, Feb 22, 2015 at 11:42 AM, Evan Tahler [email protected]
wrote:

Philosophy Ramblings:

This is awesome. However, if you you able to save a new model and re-load
it quickly, we know that it's very likely that ES will not yet have indexed
the new object, and things like model.reload! or just finding the object
again be likely to fail. This isn't anything different than how the tool
worked before, but the ActiveRecord semantics (because folks are used to
working with mySQL et al) might imply that you can think about model.save
in the same way.

Since you are working with only one record here (as opposed to multi
writes which you might use some of the other waistband sections for), you
have the opportunity to enforce that when you call model.save, the next
line really will have it available:

  • You can ask the index for its refresh frequency and sleep that long
    after a save?
  • You can do a while unreadable loop (with a 5s timeout) to try and
    read the object before moving on from the save method
  • You can cache the object in some way?

Eh, these are all probably terrible ideas... never mind!


Reply to this email directly or view it on GitHub
#18 (comment).

Pablo Jairala |* TaskRabbit *| Staff Engineer

@evantahler
Copy link
Contributor

Huh, I thought you always needed to wait for the refresh (even when loading by document ID).... Fancy!

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.

4 participants