Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Makes UNS more efficient by refactoring UNS methods in order to use the datastore once for a list of ids instead of multiple times #438

Merged
merged 6 commits into from Jan 9, 2013

Conversation

Projects
None yet
2 participants
Contributor

swarbhanu commented Dec 31, 2012

Uses read_mult() wherever possible instead of read().

The UNS API is not changed at all by this pull request.

- datastore = self.container.datastore_manager.get_datastore('events')
- event_obj = datastore.read(event_id)
- events_for_message.append(event_obj)
+ events_for_message += self.datastore.read_mult(ret_vals)
@daf

daf Jan 9, 2013

Contributor

Nitpick, use extend instead of +=.

@swarbhanu

swarbhanu Jan 9, 2013

Contributor

Cool. I pushed the change in: extend for lists instead of +=.

if item['_type'] == 'NotificationRequest':
- notif = self.clients.resource_registry.read(item['_id'])
+ object_ids.append(item['_id'])
@daf

daf Jan 9, 2013

Contributor

For a block like this, consider the use of a list comprension:

object_ids = [item['_id'] for item in ret_vals if item['_type'] == "NotificationRequest"]

Obviously they are not a panacea, they can get difficult to read with any more complexity, but for a case like this, it can actually improve readability. Don't bother updating this one but just a note for the future.

@swarbhanu

swarbhanu Jan 9, 2013

Contributor

Yes, good point. I will try to make things more pythonic .

Contributor

daf commented Jan 9, 2013

This looks great!

daf added a commit that referenced this pull request Jan 9, 2013

Merge pull request #438 from swarbhanu/use_read_mult
Makes UNS more efficient by refactoring UNS methods in order to use the datastore once for a list of ids instead of multiple times

@daf daf merged commit cef1320 into ooici:master Jan 9, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment