Closed justinhynes closed 11 months ago
Update as this came up again very recently.
The validate functionality in this library calls out to msgfmt
: https://github.com/openedx/i18n-tools/blob/851c02f7a87d1ec146a2932413b43d58ba8923d0/i18n/validate.py#L76.
Specifically, the -c
flag ensures that the format, headers, and domain directives pass. I think this is where we are running into issues. We're using msgfmt
to check the headers, which I think expects PO-Revision-Date
to be present?
https://github.com/openedx/i18n-tools/pull/129 seems to be the culprit. @shadinaif @OmarIthawi any thoughts?
@justinhynes Sorry that it broke the CI. It was an unintended consequence of our change. Otherwise, we could have bumped a major version for the edx-i18n-tools
package.
To give some context, we removed it because PO-Revision-Date
triggers a lot of diffs and commits in the openedx/openedx-translations. I still thinks this is a good change because PO-Revision-Date
has practically minimal use of in the en
source translations.
Translations is being refactored as part of OEP-58 which will eventually remove the translations from individual repositories. Credentials deprecation pull requests haven't been done but here's an example from another repo:
I'd like to suggest a couple of options in the following order:
-c
flag for the English translations in the credentials repository CI.PO-Revision-Date
for all i18n_tool extract
files e.g. PO-Revision-Date: 1970-01-01 10:30+0000
so it passes the -c
flag.i18n_tool
until further notice.Please let me know which option makes more sense to proceed with and we'll try to fix the issues.
Thanks for the extra eyes and your suggestions on this, @OmarIthawi.
Unfortunately, we can't follow your first suggestion -- the -c
flag is coming from this library itself, not specified in Credentials.
The reason this is turning into a problem now is that we were told that upgrading to 1.x of i18n-tools
is required for the Django 4.2 upgrade in the Open edX IDAs. We have Credentials pinned to a release of i18n-tools
< 1.x, but now we're blocking the Django upgrade now. I
We can try and look at suggestion #2, but I expect that the i18n tool will just strip it out again?
We can try and look at suggestion no. 2, but I expect that the i18n tool will just strip it out again?
@justinhynes This behavior will be added to i18n_tool extract
. That should pass the -c
, we'll add a test case to ensure that whatever the i18n_tool extract
generate passes the validate
command. It makes sense that both commands are "on the same page" so to speak.
Again, apologies for breaking the workflow. no. 2 seems to be the way to go. So if others agree we can move forward and document this decision as soon as possible.
cc: @shadinaif and @brian-smith-tcril what do you think?
Thank you for the details @justinhynes . I was able to reproduce the error locally. The fix is on the way. I tried it and now quality_and_translations_tests
job is passing successfully on my local machine
cc: @OmarIthawi @brian-smith-tcril
@justinhynes We've completed the fix, would you mind testing it before we cut a new release?
Hey @OmarIthawi,
I'll try to get to this today. Sorry for the slow response, I was out of commission for a few days due to COVID.
@justinhynes thanks for the heads up! Get well and fully recovered soon!
I was able to try the changes first thing this morning and it looks good. No issues locally. Thanks!
Closed by https://github.com/openedx/i18n-tools/pull/140 and released in v1.3.0.
We recently updated the
edx-i18n-tools
package in the Credentials IDA and it appears to be causing issues. We have temporarily pinned the version to the 0.9.x release (which resolved our CI issues).It looks like the
PO-Revision-Date
attribute in the file (headers?) are being removed now. When validation is run using this package, it is flagging the missingPO-Revision-Date
data as a warning, which causes the validation to fail with an error. Since the validation exits with an error code, it is causing issues in the Credentials CI.I have a PR demonstrating the issue here: https://github.com/openedx/credentials/pull/2085/files.
The failing CI checks can be seen here: https://github.com/openedx/credentials/actions/runs/5588178826/jobs/10214553458?pr=2085
Example logs from the failing
validate
: