Add Prolific provider (#1008) #1012
Conversation
facebook-github-bot
added
the
CLA Signed
There was a problem hiding this comment.
Overall structure looks good. Big fan of having the prolific_api wrapped into a useful utility. I'm not sure it should sit in base mephisto given it should only be used in mephisto core through the abstractions layer.
Added some early thoughts about Qualifications in Prolific's API, since these will likely be the hardest to map.
| @@ -84,6 +84,8 @@ tornado = { version = "^6.0", optional = false } | |||
| parlai = { version = "^1.7.0", optional = true } | |||
| torch = { version = "^1.4.0", optional = true } | |||
|
|
|||
| # jsonschema | |||
There was a problem hiding this comment.
Comment is generally the purpose for having the library. Here it's likely "api validation"?
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1012 +/- ##
==========================================
- Coverage 63.99% 60.45% -3.55%
==========================================
Files 108 155 +47
Lines 9472 11796 +2324
==========================================
+ Hits 6062 7131 +1069
- Misses 3410 4665 +1255
☔ View full report in Codecov by Sentry. |
meta-paul
force-pushed
the
add-prolific-provider
branch
from
5c2677b to
c52921c
Compare
meta-paul
force-pushed
the
add-prolific-provider
branch
from
c52921c to
87b37a3
Compare
meta-paul
force-pushed
the
add-prolific-provider
branch
7 times, most recently
from
f90ac1a to
08aa5c3
Compare
meta-paul
force-pushed
the
add-prolific-provider
branch
from
08aa5c3 to
7aa9fda
Compare
| available_balance: Optional[Union[int, float]] | ||
| balance: Optional[Union[int, float]] | ||
| balance_breakdown: Optional[dict] | ||
| beta_tester: bool |
There was a problem hiding this comment.
I don't imagine that all of these fields will end up being populated in a consistent manner, let alone relevant.
| params = workspace.to_dict() | ||
| if params["description"] == "": | ||
| params.pop("description", None) | ||
| params.pop("product", None) # TODO: What is this? Don't see this field in API docs |
There was a problem hiding this comment.
Wondering this too - I've started getting it from Workspace, but if I drop this then the API call yells at me for including an incorrect value for product.
meta-paul
force-pushed
the
add-prolific-provider
branch
from
a4e5e25 to
3d13138
Compare
| @@ -0,0 +1,42 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Now that this is all functional, I'm not sure what the correct thing for us to be doing to demonstrate different providers will be, but I don't think it's going to be to have a different script in here.
There was a problem hiding this comment.
Note that there are some Prolific-specific bits in there, like default_config_file and shared_state.prolific_specific_qualifications (it was meant to show how to work with Prolific-specific integration)
There was a problem hiding this comment.
Agreed, just I think we need to have another way to demonstrate provider use in a script, rather than leaving them in task examples (something like examples/providers which uses a simple task (ideally the React variant) and show it for multiple providers). We can commit here for now, but it's a TODO item.
| # Name wrapper after wrapped class (for logging) | ||
| Wrapper.__name__ = target_cls.__name__ |
|
|
||
| return self.db_status | ||
|
|
||
| # Remaining statuses are tracking a live Study |
There was a problem hiding this comment.
To match our intended semantics it should be tracking a live submission.
| client = self._get_client(requester.requester_name) | ||
|
|
||
| # time.sleep(2) # Prolific servers may take time to bring their data up-to-date | ||
| study = prolific_utils.get_study(client, prolific_study_id) |
There was a problem hiding this comment.
And you should write get_submission, as the statuses of a submission should align here:
No description provided.