Repository navigation
Fix/return data contracts - #114
Conversation
|
Hi @andrewtavis, could you review this when you have a moment? 😊 The contracts endpoint is working again on Toolforge. You can test it here: Both return the contracts as YAML, exactly as exported by Scribe-Data. For now, the contracts on Toolforge were copied over manually from Scribe-Data This means @angrezichatterbox can now use the contracts endpoint for Android. The root cause of this issue is #70; I didn't add this process. I was focused on removing the hardcoded contracts. |
|
Hey @DeleMike and @angrezichatterbox: I think that the changes here are almost there :) One thing I'm wondering about though is the Quick note, @DeleMike: My suggestion:
In this way we can always get the contract with individual data packs, and we have the global endpoint for all of them that could be unzipped in an application if for some reason in the future we want to get all contracts (or for instance we want to get more than one, so we zip the ones we want and send that). Does this make sense? Please let me know as I'm also not 100%! :) |
|
Thanks @andrewtavis for the feedback. I think for I will sit back later and digest all of your comments and then extend the functionality of this PR. BTW, Thanks! |
|
I'm sure it'd be fine for us to convert the contracts into a JSON file for the request and then back into YAML once they reach the clients, @DeleMike! It's just important that we maintain them as YAML files so they're more human readable. |
|
So we can of course have |
Totally agree, @andrewtavis! I will apply all suggestions here. Thank you :) |
|
I think that sending JSON for the |
|
As the clients will be hard coded to fulfill the contracts that are specific to the language, and the extra keys for |
|
No problem. I will update all my decisions here. 🙌🏾 |
|
Hi @andrewtavis , been researching this for a few hours, and when I checked I'd still add the zip at Also, currently the Android |
|
Hi @andrewtavis, I've updated this based on your suggestions:
The contracts stay as YAML on the server and are only converted when sent. Please note that Toolforge will show the new responses once this is merged and the server is rebuilt. Please test on local. What do you think? See example of output:
|
Thanks for bringing this up, @DeleMike! We have scribe-org/Scribe-Android#715 that's fixing scribe-org/Scribe-Android#714 that should finalize this once this PR is in 😊 |
andrewtavis
left a comment
There was a problem hiding this comment.
praise: All's looking amazing here, @DeleMike! 🤩 Can't wait to see how this goes in scribe-org/Scribe-Android#715! 😊
|
Glad to have this in! |
Contributor checklist
./pre-commitexecutable as well asmake lintand have fixed all reported issuesDescription
update_data.shnow copies the exported contracts into the server'scontracts/folder. They're copied into a temporary folder first, so a failed copy keeps the current contracts.update_scribe_data.ymlalso packages the contracts with the SQLite packs, uploads both as the artifact, sends both to Toolforge, and replaces./contractsduring deployment. A failed extraction keeps the current contracts./contractsreturns503 "Data contracts are not available"if the contracts are missing, instead of a generic 500.contracts_en.yaml,contracts_de.yaml, orcontracts.yamlfor all languages.Testing
update_data.shstep with realscribe-data ecoutput, and the Toolforge extraction with a good and a corrupt package./contracts,?lang=enand?lang=dereturn 200 with the expected file names, and 503 without contracts.Related issue