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

Proposed resolution for issue #19 #72

Merged
merged 1 commit into from
Jun 14, 2016
Merged

Proposed resolution for issue #19 #72

merged 1 commit into from
Jun 14, 2016

Conversation

stewart-r
Copy link
Contributor

I think this resolves the issue without having to worry about continuation tokens/etags.

The Azure SDK seems to split it up into segmented queries when we call .Execute() in this class:

internal IEnumerable<DynamicTableEntity> Execute(CloudTableClient client, string tableName,     TableRequestOptions requestOptions, OperationContext operationContext)
    {
        CommonUtility.AssertNotNullOrEmpty("tableName", tableName);
        TableRequestOptions modifiedOptions = TableRequestOptions.ApplyDefaults(requestOptions, client);
        operationContext = operationContext ?? new OperationContext();

        IEnumerable<DynamicTableEntity> enumerable =
            CommonUtility.LazyEnumerable<DynamicTableEntity>(
            (continuationToken) =>
            {
                Task<TableQuerySegment> task = Task.Run(() => this.ExecuteQuerySegmentedAsync((TableContinuationToken)continuationToken, client, tableName, modifiedOptions, operationContext));
                task.Wait();

                TableQuerySegment seg = task.Result;

                return new ResultSegment<DynamicTableEntity>((List<DynamicTableEntity>)seg.Results) { ContinuationToken = seg.ContinuationToken };
            },
            this.takeCount.HasValue ? this.takeCount.Value : long.MaxValue);

        return enumerable;
    }

I wasn't 100% confident on my navigation of the SDK source so I double checked by temporarily adding an integration test to insert 10,000 rows into a partition and confirm they had been deleted after calling the proposed new function. The test worked as expected (albeit slowly). I have not added this test to the main branch because it slowed down the integration tests but you can see the approach here: https://github.com/stewart-r/AzureStorageTypeProvider/tree/delete-partition

This blog seems to imply it is an inherently slow operation. I can switch to using the approach in the blog and test the batch-size to try to trim down the time if you think it would be worthwhile?

(sorry about that git spaghetti - I got rid of the erroneously included files in the end but at the expense of an armada of commits I am now scared to try to squash/rebase! :-) It only appears as a single commit in the main repo if the pull requested is accepted, right?).

@isaacabraham
Copy link
Collaborator

I believe the full commit history will be ported across - I'm not sure about how to do squashing myself either! Let me check, but yes, it'd be great to just get the single commit without the binaries in the history.

@isaacabraham
Copy link
Collaborator

But the PR looks great otherwise :-)

@stewart-r
Copy link
Contributor Author

It keeps it all? Haha oh no, guess it was inevitable! Ok, let me sort it out.

Please don't lose any time on untangling my git mess Isaac. It's my mess, I'll sort it out! Think I've got to grips with it now anyways.

I'll have a bash at squashing it down shortly.

@isaacabraham isaacabraham merged commit e1b5678 into fsprojects:master Jun 14, 2016
@isaacabraham
Copy link
Collaborator

Thanks!

@stewart-r
Copy link
Contributor Author

My pleasure!

So sorry about all the git messiness!

Think the issue is that I pulled some of your upstream changes too but in the middle of my series of commits so there was a commit from me, then a merge of your upstream changes, then another from me etc. This would probably have been ok until I mistakenly included those build files then started trying to revert, rebase and reset my way out of trouble! :-/

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