UPDATE: preload yaml

[hey-leon]

After an incident that was causing a failed CI pipeline on one of our production applications. We noticed there were some missed performance opportunities in the architecture of this class.

Previously we were essentially loading all translation files per source edit. These changes essentially preloads them and then works through the edits from there.

[MichaelHoste]

Hi @leonp1991,

Thanks for this issue, we're looking at it!

Previously we were essentially loading all translation files per source edit.

Actually, there is a reason for this. After each loop, one of the YAML files is updated with the new source text of the source edit. So we load them all again to have the latest version (not optimal, I agree...).

The reason is that If many source edits are related to the same key, we need the latest version of the YAML files to be able to have the correct condition if value == source_edit['old_text'].

I'm sure we can find a way to improve this code and we're looking at it. But I have a question : why do you have so many source edits in your request? Is the .translation_io file present on your CI? You need it with the correct timestamp to be able to sync "only" the last source edits.

[hey-leon]

Hi @MichaelHoste as far as I could tell It looks like the code always updates the in memory hash before writing to file. meaning if we continually use the in memory hash we will always be writing the latest version of the file. I have not looked at the test suite though, so Im not sure if there a case thats not tested that needs to be added.

As for why we have needed so many edits. We are transition a product from containing static text, to text that is dynamic based on a users configuration of the product. This has lead to bulk key updates.

If it counts for anything we have been using this fork since this PR was opened.

[MichaelHoste]

Hi @MichaelHoste as far as I could tell It looks like the code always updates the in memory hash before writing to file. meaning if we continually use the in memory hash we will always be writing the latest version of the file. I have not looked at the test suite though, so Im not sure if there a case thats not tested that needs to be added.

I noticed this and that's why consecutive source_edits on the same key still work well with your pull request (this was tested in the specs!).

But there is this untested part that doesn't work well with the pull request: https://github.com/translation/rails/blob/master/lib/translation_io/client/sync_operation/apply_yaml_source_edits_step.rb#L39

It's a tricky part that allows you to create a source_edit for a key that comes from a gem directory (will_paginate, devise, etc.). And instead of editing the YAML file inside the gem directory (bad!), it creates a new key on your source YAML file to override the one of the gem (good!).

This part still works with your pull request but only if there is only one source edit for the same "gem key" on a single sync. But we need to still make it work for several source edits.

Tomorrow, I'll add a test for this very specific case and I'll try to refactor this service to make it work with your changes.

Thanks for your help!

[hey-leon]

Thanks for that information @MichaelHoste. I appreciate you taking the time to explain that. Great news on the refactor. Look forward to it.

[MichaelHoste]

Hi @leonp1991,

I added the missing specs here: 5e67aae#diff-a2df255f94ce27d98252df949170957bR122

And I refactored the service so that the YAML loading is only triggered when needed (after adding a new YAML file that override a gem YAML). Here is the new code: https://github.com/translation/rails/blob/master/lib/translation_io/client/sync_operation/apply_yaml_source_edits_step.rb

If these changes seem ok to you, I'll bump the Gem version tomorrow.

Thanks again for your help!

[hey-leon]

Looks good, happy to be moving back from the fork. Thank-you.

[MichaelHoste]

Looks good, happy to be moving back from the fork. Thank-you.

You should be able to move back from the fork, using Version 1.21.

Thank you for your help with this!