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

Resolve MTurkUnit expiration bugs #382

Merged
merged 2 commits into from
Jan 29, 2021
Merged

Resolve MTurkUnit expiration bugs #382

merged 2 commits into from
Jan 29, 2021

Conversation

JackUrb
Copy link
Contributor

@JackUrb JackUrb commented Jan 27, 2021

Overview

A combination of two bugs was leading to improper outcomes when expiring a task with Ctrl-C. First, MTurkUnit wasn't properly updating from the CREATED status to the ASSIGNED status unless a db status load occurred elsewhere. The second issue was that moving from ASSIGNED to EXPIRED didn't trigger the same return event that it did when moving from ASSIGNED to LAUNCHED, preventing running assignments from marking and exiting properly. The combination of these two bugs led to some completed tasks being locally marked as expired, and preventing some workers from being able to submit complete work after a Ctrl-C was triggered.

Resolution

To resolve the lack of CREATED state change, MTurkUnit's get_status method now calls the super method in when currently in CREATED, forcing that refresh using the base logic.
For triggering a RETURNED event, the logic for this is extended to include the transition from ASSIGNED to EXPIRED, however the new transition should still set the unit's db status (rather than deferring to the blueprint to decide whether to expire or make available again) as we know the outcome is EXPIRED.

Testing

Launched tasks in MTurk sandbox, noted I could now get a proper return event after expiring hits, and could properly exit on a double-return event. Unit tests still passing

@JackUrb JackUrb requested a review from jxmsML January 27, 2021 23:02
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 27, 2021
Copy link

@jxmsML jxmsML left a comment

Choose a reason for hiding this comment

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

👍 🚀 thanks for fixing this so fast!!

@jxmsML
Copy link

jxmsML commented Jan 29, 2021

is this one good to merge?

@JackUrb JackUrb merged commit 1a37f05 into master Jan 29, 2021
@JackUrb JackUrb deleted the mturk-expired-bugs branch January 29, 2021 16:52
@JackUrb JackUrb restored the mturk-expired-bugs branch July 8, 2021 16:20
@JackUrb JackUrb deleted the mturk-expired-bugs branch July 8, 2021 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants