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

workflows: fixes to make author dags work #3

Merged
merged 1 commit into from
Jul 10, 2024
Merged

Conversation

DonHaul
Copy link
Collaborator

@DonHaul DonHaul commented Jul 1, 2024

No description provided.


data = {
"$schema": "http://inspirebeta.net/schemas/records/authors.json",
"name": {"value": "PlaceHolder"},
Copy link
Contributor

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?

Copy link
Collaborator Author

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

Copy link
Contributor

@drjova drjova left a 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

Comment on lines 99 to 103
data["name"]["value"] = (
workflow_data["data"]["data"]["given_name"]
+ " "
+ workflow_data["data"]["data"]["family_name"]
)
Copy link
Contributor

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed

Copy link
Contributor

@drjova drjova left a 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"""
Copy link
Contributor

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"},
Copy link
Contributor

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?

@DonHaul DonHaul force-pushed the main branch 2 times, most recently from 85fa478 to 1fe1204 Compare July 8, 2024 11:55
@drjova
Copy link
Contributor

drjova commented Jul 8, 2024

@DonHaul could you please squash?

@DonHaul
Copy link
Collaborator Author

DonHaul commented Jul 8, 2024

@drjova could you give me write access? I cant seem to merge

@DonHaul DonHaul force-pushed the main branch 3 times, most recently from 3771821 to 5e71e8f Compare July 8, 2024 16:16
@drjova
Copy link
Contributor

drjova commented Jul 9, 2024

@DonHaul could you please rebase?

@drjova drjova merged commit 5f48746 into inspirehep:main Jul 10, 2024
1 check passed
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.

2 participants