-
Notifications
You must be signed in to change notification settings - Fork 327
Add support for ExportItems and UploadItems #109
base: master
Are you sure you want to change the base?
Conversation
Hi @richard-einfinity, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution! TTYL, AZPRBOT; |
@richard-einfinity, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
Hi eInfinity, how are you? Best Regards |
Hi @flugaoveltem, What are you looking for the code for the implementation or a sample of the implementation being used? Kind regards Richard |
I'm looking for the code for implementation, Thanks |
Hi @flugaoveItem,
The code for the implementation is in our fork. Compare the branch to the main project branch for the implementation code.
Cheers
Richard
…Sent from my iPhone
On 29 May 2017, at 13:50, flugaoveltem <[email protected]<mailto:[email protected]>> wrote:
Hi @richard-einfinity<https://github.com/richard-einfinity>,
I'm looking for the code for implementation,
Thanks
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#109 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AI891a9uq3DB-CBbvMKzYtlw3ZBChT-7ks5r-r8pgaJpZM4MRXK8>.
|
Sorry for the inconvenience @richard-einfinity. Im looking for a sample of the implementation being used thanks. |
Hi @flugaoveltem, See below
|
Thanks. You are awsome. |
Hi Richard, Thanks for the great commit! I've been testing out your implementation in my own code here, and I think I have found a memory leak. At least I seem to have eliminated my code as the cause of this leak, what I see essentially is that when using ExportItems all items remain in memory for that thread until the thread ends. I haven't tested with your same code above, but the main difference in my implementation is that after calling ExportItems I write the byte[] array data to disk with a simple File.WriteAllBytes(tempFile, data) call. At that point a reference to the data bytes is left open somewhere and eventually if the folder being exported contains more items than memory permits eventually an out of memory exception occurs. Specifically if I do NOT write the bytes to disk after calling ExportItems then the memory is released normally. Can you confirm if this occurs for you? Quick follow up: I've adapted my code to use the Byte[] array in memory just as you do without any writing / reading from disk and I have the same issue, so I assume your code example above also will. Thanks, |
Hi @martinlaukkanen, I haven't seen this issue previously. Can you give me a sample of the code? Cheers Richard |
Hi Richard, Thanks for following up. I took a look at your sample above to see if I can replicate my issue, and initially your sample code worked without the issue. However to match my use case I modified it as below and was able to reproduce the issue. Better yet I was also able to solve it! Basically the difference was that I am creating a Folder and referencing it by Folder.Id property rather than using a WellKnownFolderName, this is how I work with Item.Bind and such without similar issues. The issue was in the Folder.Id reference on the UploadItem construction, as you can see in my comments in the code referencing the parent folder by: ParentFolderId = new FolderId(folder.Id.UniqueId) is the only way to avoid the memory leak. `using System; namespace TestEwsExportItems
} |
Core/ExchangeService.cs
Outdated
/// </summary> | ||
/// <param name="item">The item to Upload</param> | ||
/// <returns></returns> | ||
public UploadItemsResponse UploadItem(UploadItem item) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the empty lines to follow the convention in the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@richard-einfinity would be nice if you could fix that and sign the CLA again. We may get this into the official repo then. Thx!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Return description updated and CLA resigned.
@davster ready for merge! |
No description provided.