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

Future of Import Locales and Sentence Collector export #3636

Closed
MichaelKohler opened this issue Mar 26, 2022 · 3 comments · Fixed by #3658
Closed

Future of Import Locales and Sentence Collector export #3636

MichaelKohler opened this issue Mar 26, 2022 · 3 comments · Fixed by #3658
Assignees
Labels

Comments

@MichaelKohler
Copy link
Member

https://github.com/common-voice/common-voice/blob/main/package.json is missing the mysql dependency, leading to a broken Sentence Collector export: https://github.com/common-voice/sentence-collector/runs/5703458747?check_suite_focus=true . Even with that fixed, the Sentence Collector export would probably still fail as it does not have access to the MySQL credential secrets.

What is the plan for the future for this script? Is it still meant to be run during export? If so, you'd need to add the credentials with the right env vars to the Sentence Collector repo. If not, then we need to come up with another way to periodically run this script.

@MichaelKohler
Copy link
Member Author

const { getConfig } = require('../server/js/config-helper'); is also rather unfortunate for that kind of setup. Would love to hear what the end goal is here :)

@mozgzh
Copy link
Contributor

mozgzh commented Mar 28, 2022

Ah interesting, I didn't realize import-locales was being called from SC's actions and exported outside the repo - I assumed it was being called internally within CV before deploy, and would have access to certain env variables. As for why these changes are necessary, there's a need to have CONTRIBUTABLE_MIN_SENTENCES differ per language (instead of just 5000) and have the frontend also reflect said changes. See #3614 for more info regarding the merge for backend changes and #3477 for frontend changes.

In the medium-term, ideally contributable.json would be replaced with a db query that's able to dynamically return the list of contributable locales and generally, everything starts moving in the db instead of across static files when it makes sense.

Just brainstorming here, but I wonder if it might make sense to remove that section for the GH action (the one that runs import-locales) and have that run on app instantiation or scheduled job instead. That's assuming the rest of the export isn't dependent on changes from the import-locales script. I'd love to get more insight into the export and general feedback.

@MichaelKohler
Copy link
Member Author

@mozgzh thanks for the answer. Generally there is absolutely no other dependency from the Sentence Collector export side. It was simply added there as it didn't run anywhere else and at that point it made sense to run it after exporting. For now I've removed that step. This means that currently contributeable.json won't be updated anymore and there is no other mechanism in place to know that we'd need to do manually. So yes, it would be good if you could integrate this either in the deploy process or a cron on the CV side. I'll leave it up to you to decide what way forward you'd like to go :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants