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

Post-Octopus chore: remove temporary changes #30

Merged
merged 5 commits into from
Jun 20, 2024
Merged

Post-Octopus chore: remove temporary changes #30

merged 5 commits into from
Jun 20, 2024

Conversation

colleenXu
Copy link
Contributor

@colleenXu colleenXu commented Jun 18, 2024

For the Octopus patch and biothings/biothings_explorer#811

@tokebe, does the smartapi overrides yaml look okay? Does it work correctly with the smartapi_sync cron job?

I don't often write empty objects (apis).
And I tried manually triggering smartapi_sync locally with API_OVERRIDE=true, and I get a console log saying the update failed, which is maybe correct behavior...

@tokebe
Copy link
Member

tokebe commented Jun 18, 2024

Errors typically aren't correct/intended behavior...I've pushed a commit that specifies an empty list.

@colleenXu
Copy link
Contributor Author

colleenXu commented Jun 19, 2024

Hmmm the apis value is an object with key:value pairs when there are overrides.

Would an empty object {} make more sense here/remind devs of the format for overrides, or is the empty list fine? It does look like the cron job works without errors with the current empty list.....

@tokebe
Copy link
Member

tokebe commented Jun 20, 2024

apis is a list of objects (hence the - in front of entries). The override objects have key: value pairs, but are contained as a list. That's why the cron job runs without errors; it's expecting a list of objects (see the TypeScript types).

Crossed my wires on this, fixed in this commit

@tokebe tokebe merged commit 5d9a277 into main Jun 20, 2024
0 of 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.

2 participants