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

Issue #1587: Fixes a bug in the JSON API for jobs #1588

Merged
merged 3 commits into from
Dec 5, 2024

Conversation

exoticDFT
Copy link
Member

With the current jobs.yml, we are missing a , after our last job object in our generated JSON at https://us-rse.org/api/jobs.json.

Description

The JSON API for the jobs occasionally misses a , after our last jobs object due the to way we sort. It seems to happen if the last non-expired job in our jobs.yml has a posted date that is earlier than other jobs that are not expired. This should fix #1587 by enforcing a , at the end of every object in the JSON and ending the array with an empty object i.e. {}.

  • Adds a , for every element in our jobs array
  • Updates formatting to see the liquid logic more clearly
  • Moves nowunix capture outside of the for loop for jobs

Checklist:

  • I have previewed changes locally or with CircleCI (runs when PR is created)
  • I have completed any content reviews, such as getting input from relevant working groups. If no, please note this and wait to post the PR to the #website channel until the content has been settled.

When you are ready for a technical review/merge, post the for the link for the PR in the US-RSE Slack (#website) to ask for reviewers.

exoticDFT and others added 2 commits September 26, 2024 15:43
- Adds a `,` for every element in our jobs array
- Updates formatting to see the liquid logic more clearly
- Moves nowunix capture outside of the for loop for jobs
@crd477
Copy link
Contributor

crd477 commented Nov 18, 2024

Copy link
Contributor

@crd477 crd477 left a comment

Choose a reason for hiding this comment

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

This LGTM. Is it OK by you to merge it?

@crd477 crd477 merged commit d8cebf4 into USRSE:main Dec 5, 2024
2 checks passed
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.

Issue with JSON API for the jobs
2 participants