linkedin / pyexchange

Python wrapper for Microsoft Exchange
Apache License 2.0
153 stars 98 forks source link

Add list_events capability to search between two dates #15

Closed ematthews closed 9 years ago

ematthews commented 9 years ago

I have attempted to add the guidelines as well, however I am certain that I am missing something or the tests are not thorough enough. This is intended to allow a user to request all calendar items between a start and end python datetime value. The result will be a list containing individual Exchange2010CalendarEvent items.

catermelon commented 9 years ago

Thank you so much for the contribution - let me look through it and get back to you. I'm going to be away at a conference, so communication might be a little slow the next couple of days.

got-root commented 9 years ago

The FindItem operation does not return some fields, like 'Body', even if you specify 'AllProperties' in the BaseShape. I would recommend combining the FindItem with CalendarView just like you did, but switch the BaseShape to 'IdOnly'. Then do a GetItem operation and include all of the ID's returned from the FindItem operation. I somewhat started doing this before and ran into that problem.

ematthews commented 9 years ago

@got-root I definitely agree with you. I initially had it automatically retrieving full details but realized that I was doing quite a number of requests unnecessarily. The use that I initially was looking for was to simply get the "highlights" about calendar items (specifically start/stop, subject line, and the actual ID itself) so that we could get further details about specific items.

In order to populate everything, including Body, the load_all_details() function (https://github.com/linkedin/pyexchange/pull/15/files#diff-36db3bb087cec28d50a3c4f58fb9fb24R133) was added to address this problem. I personally thought that keeping it as an optional command reduced overall load on the Exchange server when you don't need the additional details.

Thanks!

got-root commented 9 years ago

Oh cool! I missed that method. I like the idea of making it optional. However, your method does a request for each event which would make n+1 requests when you include the FindItem. You can actually send all of the event's ID's in a single GetItem request. So the FindItem + the GetItem would only be 2 total requests for n events.

ematthews commented 9 years ago

Oh! Now we are both missing something :). I didn't realize that I could send the list of event IDs in a single request. I'll try to update that later today and tomorrow in the request. Thanks!

got-root commented 9 years ago

Awesome! :+1:

catermelon commented 9 years ago

Hey @ematthews did you update the pull request? Github doesn't tell me. I wanted to review and get it in today if possible. Thanks again!

ematthews commented 9 years ago

Sadly, I have not been able to work on it in the past few days. I will try to take care of it tomorrow. Thanks for the follow up and apologies on the delay!

catermelon commented 9 years ago

No rush at all - just so you know, I'm doing some behinds the scenes housekeeping - I'm moving over to pytest from nose, and I found out I can ditch urlib2 for requests (yay). Those are all going in right now.

Just ping me whenever you're done so I know to take a look. Thanks again!

On Tue, Oct 14, 2014 at 3:21 PM, Eric Matthews notifications@github.com wrote:

Sadly, I have not been able to work on it in the past few days. I will try to take care of it tomorrow. Thanks for the follow up and apologies on the delay!

— Reply to this email directly or view it on GitHub https://github.com/linkedin/pyexchange/pull/15#issuecomment-59127111.

ematthews commented 9 years ago

@trustrachel I think it's good to go now. I did update the (minimal) associated tests to use pytest so it seems to be happy with Travis. I also realized that this kind of relates to your issue #1 in subject but not really in body. I think you're request in #1 is related to finding free time, as opposed to finding what is already on the calendar which is what this pull request is hoping to address. Thanks again!

catermelon commented 9 years ago

:+1: