Skip to content

Conversation

@erogluorhan
Copy link
Member

@erogluorhan erogluorhan commented Dec 22, 2023

Closes #369

This PR addresses the needs due to version changes in pydantic as suggested in the pydantic Migration Guide:

  • Remove the deprecated __post_init_post_parse__() and move its functionality into _get_inputs()
  • Use model_dump_json() instead of deprecated dict()
  • Also uses "affiliation" instead of formerly used "institution" in gallery_generator.py, resource_gallery.yaml, and collect-user-submission.py files. This change also needed the ProjectPythia/.github/PR#63

Note to reviewers: See

@github-actions
Copy link
Contributor

github-actions bot commented Dec 22, 2023

👋 Thanks for opening this PR! The Cookbook will be automatically built with GitHub Actions. To see the status of your deployment, click below.
🔍 Git commit SHA: f9cce37
✅ Deployment Preview URL: https://projectpythia.github.io/_preview/381

@erogluorhan erogluorhan marked this pull request as ready for review December 26, 2023 20:01
@erogluorhan erogluorhan requested a review from a team as a code owner December 26, 2023 20:01
@erogluorhan erogluorhan requested review from dcamron and ktyle and removed request for a team December 26, 2023 20:01
affiliation=item.get('affiliation'),
affiliation_url=item.get('affiliation_url'),
affiliation=item.get('institution'),
affiliation_url=item.get('institution_url'),
Copy link
Contributor

Choose a reason for hiding this comment

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

A thought: should we update the form template and card text to all use "Affiliation" regardless? It seems more broadly applicable than "Institution" and is in-line with expected fields in eg CITATION.cff.

Copy link
Member Author

@erogluorhan erogluorhan Dec 26, 2023

Choose a reason for hiding this comment

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

Yes, I agree! We can do that in conjunction with this PR

Copy link
Member Author

@erogluorhan erogluorhan Dec 26, 2023

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

...and also the gallery_generator.py and resource_gallery.yaml files here for "institution" to "affiliation" changes

@erogluorhan
Copy link
Member Author

Note to reviewers:

This PR should be complete; and input submission validation should likely be running successfully. I am not 100% sure because when I bypassed branch protections and merged a prior PR into the main (FYI: later reverted) to see the things because testing this locally is quite hard and testing without merging into main is impossible, there was a serialization error at the final line of collect-user-submission.py, i.e. json.dump(inputs, f). See this

After that, this PR uses inputs = issue.submission.model_dump_json() rather than inputs = issue.submission.model_dump().

To test similar things locally, I used a sample github event payload file that had an issue creation event in it but could not replicate this serialization issue. I suggest that we merge this into main and see how it behaves, if things still don't get validated, we can follow up with an additional PR.

@dcamron
Copy link
Contributor

dcamron commented Dec 27, 2023

Agreed! Thanks Orhan.

@dcamron dcamron merged commit 24ba061 into main Dec 27, 2023
github-actions bot pushed a commit that referenced this pull request Dec 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Resource gallery submission action is broken

3 participants