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

Restore RemoteCatalogEntry.metadata. #272

Merged
merged 2 commits into from
Feb 20, 2019

Conversation

danielballan
Copy link
Member

@danielballan danielballan commented Feb 19, 2019

In intake 0.4.1, entry.metadata works the same on local and remote catalogs: it fires implicit entry.get() and thus accesses entry.get().metadata.

On current master, local catalog entries still work that way, but remote catalog entries do not. This is because in f1c61db I wrongly changed self._metadata to self.metadata and in effect shadowed the "implicit get()" mechanism.

It seems worth reviewing at some point whether self._metadata should exist at all, but in the meantime I think this reversion should be merged to get back to the behavior in 0.4.1. I have added a test to protect against making the same mistake in the future.

In intake 0.4.1, ``entry.metadata`` works the same on local and remote
catalogs: it fires implicit ``entry.get()`` and thus accesses
``entry.get().metadata``.

On current master, local catalog entries still work that way, but remote
catalog entries do not. This is because in
f1c61db I wrongly changed
`self._metadata` to `self.metadata` and in effected shadowed the
"implicit ``get()``" mechanism.

It seems worth reviewing at some point whether `self._metadata` should
exist, but in the meantime I think this reversion should be merged to
get back to the behavior in 0.4.1. I have added a test to protect
against making the same mistake in the future.
@martindurant martindurant merged commit 542bd6d into intake:master Feb 20, 2019
@danielballan danielballan deleted the entry-metadata branch February 21, 2019 00:23
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.

2 participants