Skip to content
This repository was archived by the owner on Apr 30, 2024. It is now read-only.

Add AutoContext to automatically insert tasks in batches#105

Merged
7 commits merged into
Workiva:masterfrom
ericolson-wf:auto_context
Aug 13, 2013
Merged

Add AutoContext to automatically insert tasks in batches#105
7 commits merged into
Workiva:masterfrom
ericolson-wf:auto_context

Conversation

@ericolson-wf

Copy link
Copy Markdown
Contributor

Automatically insert asyncs in batches as they are added to the context instead of waiting until the context is closed.
Also allows custom batch size.

Here is a PR that uses add_async() with AutoContext: ericolson-wf#4

Intended to automatically insert asyncs in batches (asynchronously) as
they are added to the context.
Move most of handle_tasks() into _handle_tasks() to allow
AutoContext to insert some tasks before fully exiting.

Also add a parameter to allow different batch_sizes.
Also ensure they are added correctly in batches.
When batch_size is None, AutoContext will act like Context, where all
tasks are inserted at once when the context is exited.

Ensujre that when no tasks are added, there are no errors.
@ericolson-wf

Copy link
Copy Markdown
Contributor Author

@robertkluin-wf @beaulyddon-wf @tannermiller-wf @johnlockwood-wf @tylertreat-wf @mattheweverett-wf
Can anyone who's interested take a look at this?

@tylertreat-wf

Copy link
Copy Markdown
Contributor

I think it looks good. It'd be nice to add a kwarg to the new helper to create AutoContexts, e.g. context.new(auto_add=True).

@tannermiller-wf

Copy link
Copy Markdown
Contributor

I don't wonder why we don't just build this into the Context as an option and not create a sub class. And then do like Tyler suggests.

@ericolson-wf

Copy link
Copy Markdown
Contributor Author

The auto_add sounds good.
Merging with Context sounds good too, although auto inserting is a little more "magic" than the current Context. It probably depends on if the furious guy feels it goes reasonably in Context.

@ghost

ghost commented Aug 13, 2013

Copy link
Copy Markdown

I do think it might make sense to add the functionality to the core context. It shouldn't add that much code. What do you think @ericolson-wf?

Also, I can't merge this. Looks like there might be a merge conflict?

Conflicts:
	furious/context/context.py
ghost pushed a commit that referenced this pull request Aug 13, 2013
Add AutoContext to automatically insert tasks in batches
@ghost ghost merged commit aad6c7b into Workiva:master Aug 13, 2013
@ghost

ghost commented Aug 13, 2013

Copy link
Copy Markdown

I merged this. But, the more I've thought about it, the more I think this might be just as well being in plain old Context.

@ericolson-wf

@beaulyddon-wf

Copy link
Copy Markdown
Contributor

+1

This pull request was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants