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

insert_tasks_ignore_duplicates - New _insert_tasks_ignore_duplicate_name... #162

Conversation

markshaule-wf
Copy link
Contributor

...s to gracefully handle DuplicateTaskNameError exceptions raised by inserts.

@beaulyddon-wf @tannermiller-wf @robertkluin

FYI: @erikpetersen-wf @jasonaguilon-wf @macleodbroad-wf

  1. Let me know if you guys would prefer to just modify the existing _insert_tasks function, and add another option.
  2. Should we add an options that just selects the _insert_tasks_ignore_duplicate_names automatically, which shields the implementation from the user.

Right now a typical usage would be:

    with context.new(batch_size=100,
                     insert_tasks=insert_tasks_ignore_duplicate_names) as ctx:
      ctx.add(...) 

…ames to gracefully handle DuplicateTaskNameError exceptions raised by inserts.
@beaulyddon-wf
Copy link
Contributor

I think it's fine passing in the method like this. One thing we may want to do though is move this new method out of the contexts file and create a module for task insertion handling. That way we can give users a module with some different choices for handling the task insertion without having to dig through Furious internals.

@markshaule-wf
Copy link
Contributor Author

@beaulyddon-wf - Agreed - it doesn't feel right to be using an _insert_... directly.
Perhaps a new file in extras package, and we can rename to insert_tasks_ignore_duplicate_names

@beaulyddon-wf
Copy link
Contributor

yeah. or insert_task_handlers or something to that effect. In case we want to build out other methods. But yeah extras is a good spot for it.

@markshaule-wf
Copy link
Contributor Author

@beaulyddon-wf - Moved new function to furious/extras/insert_task_handlers.py

@beaulyddon-wf
Copy link
Contributor

Cool. I like that. +1

@tylertreat-wf
Copy link
Contributor

+1

@@ -0,0 +1,91 @@
from mock import Mock
from mock import patch

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in python 2.7 mock isn't part of the standard library.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright - I'll move down and group with google.

@macleodbroad-wf
Copy link

+1

1 similar comment
@tannermiller-wf
Copy link
Contributor

+1

…task_handlers.py, and correct extra/missing blank lines
@markshaule-wf
Copy link
Contributor Author

@macleodbroad-wf - comments addressed regarding imports and blank lines.

@macleodbroad-wf
Copy link

+1

1 similar comment
@tannermiller-wf
Copy link
Contributor

+1

…ameError on retry of individual task. The potential exception, TaskAlreadyExistsError, is already handled by _insert_tasks.
@macleodbroad-wf
Copy link

+1

beaulyddon-wf added a commit that referenced this pull request Jun 2, 2015
…ate_names

insert_tasks_ignore_duplicates - New _insert_tasks_ignore_duplicate_name...
@beaulyddon-wf beaulyddon-wf merged commit 06beebd into Workiva:master Jun 2, 2015
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.

5 participants