-
Notifications
You must be signed in to change notification settings - Fork 3
workflows: fixes to make author dags work #3
Conversation
|
||
data = { | ||
"$schema": "http://inspirebeta.net/schemas/records/authors.json", | ||
"name": {"value": "PlaceHolder"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need a placeholder?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the name property must have a value, it must be a non empty string
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I have added some comments
data["name"]["value"] = ( | ||
workflow_data["data"]["data"]["given_name"] | ||
+ " " | ||
+ workflow_data["data"]["data"]["family_name"] | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, two small comments
@@ -48,6 +47,7 @@ def author_create_approved_dag(): | |||
|
|||
@task() | |||
def set_workflow_status_to_running(**context): | |||
"""sets status to running""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we don't need this docstring
|
||
data = { | ||
"$schema": "http://inspirehep.net/schemas/records/authors.json", | ||
"name": {"value": "NAME"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need that?
85fa478
to
1fe1204
Compare
@DonHaul could you please squash? |
@drjova could you give me write access? I cant seem to merge |
3771821
to
5e71e8f
Compare
@DonHaul could you please rebase? |
No description provided.