Skip to content

A few improvements to getting metadata for DataDictionary feeds. - #156

Merged
dougcole merged 3 commits into
estately:masterfrom
jondruse:master
Sep 23, 2015
Merged

A few improvements to getting metadata for DataDictionary feeds.#156
dougcole merged 3 commits into
estately:masterfrom
jondruse:master

Conversation

@jondruse

Copy link
Copy Markdown
Contributor

No description provided.

When using a RETS feed with DataDictionary support, sometimes
StandardNames are used and therefore need to be accessible in the
Metadata representation.
Without this patch, if a single metadata type fails, the entire metadata
is inaccessible. It's better to rescue and keep going to give access to
what metadata is available.
Comment threadlib/rets/client.rb Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the reasoning behind this? Rescuing errors and returning data makes me a little nervous that we'll hide errors and end up with corrupt data.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Well, I agree that hiding errors can be nasty, but in this case, while iterating over Metadata::METADATA_TYPES any error renders the Client#metadata call useless because the exception bubbles all the way back up.

We either need a way of specifying which types to get, or skipping the types that encountered an error and returning what was retrieved successfully.

I should note too that when I first encountered this, I reached out the MLS support staff and they basically replied with, "yeah, we don't have that type". So there was no real solution except to basically just ignore failures.

This was the smallest change I could make to get it to work so I figured I would see if you'd be interested in putting in. I'm totally open to suggestions. Thanks.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you create a new exception class to rescue just this error? I'm worried of all the other errors we would end up hiding. Some mlses only allow one connection and this error is raised if you try and establish a second connection. I'm pretty sure there are other legitimate errors that we would be hiding too, that's just the first one that comes to mind.

some RETS feeds don't provide meta data all types defined in
Metadata::METADATA_TYPES so this provides the ability to specify which
ones to get to avoid errors or tricky error handling.
@jondruse

Copy link
Copy Markdown
ContributorAuthor

Ok I've removed the error rescuing in favor of just being able to provide the array of types to get and It can be the end user's responsibility to avoid types the MLS doesn't provide.

@dougcole

Copy link
Copy Markdown
Contributor

Cool, thanks @jondruse I like that a lot better. There is a weird edge case where the metadata caching doesn't respect the types argument (it'll just return which ever types you requested the first time), but I don't expect it'll cause problems for many people. I'll merge this and then open an issue to fix that.

dougcole added a commit that referenced this pull request Sep 23, 2015
A few improvements to getting metadata for DataDictionary feeds.
@dougcole
dougcole merged commit cff938b into estately:masterSep 23, 2015
dougcole added a commit that referenced this pull request Sep 23, 2015
Sign up for freeto 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.

2 participants

@jondruse@dougcole