Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Custom storage models #389

Closed
Gladear opened this issue Aug 9, 2024 · 11 comments
Closed

Custom storage models #389

Gladear opened this issue Aug 9, 2024 · 11 comments

Comments

@Gladear
Copy link
Contributor

Gladear commented Aug 9, 2024

Hi !
I've been wondering how feasible it would be to have different storage models in gettext - what I call a storage model is what the "compiler" module would be right now, a "compiled" storage model.

End-goal

We've been having an issue with compilation with gettext. It basically corresponds to issues #330. When we change our .po files (which happens pretty often), we need to recompile a lot of files, which makes changing gettext files pretty painful during development.

Proposition

I think a way to fix this issue is to allow users to create their own storage models. This was mentioned in the original post of #215 (see 5) but didn't seem to make it through.

I could load the translations in :persistent_term when the app starts. The backend module wouldn't need to be recompiled every time the .po files change, and therefore the rest of the app wouldn't either.
However, one may question whether this would impact performances in production (and it most likely would), so being able to use my :persistent_term storage model in dev and test, and the default "compiled" storage model in production would be a must-have.

Note that having custom storage models could also remove the need for custom logic for runtime translations (see #305).

Before going further into we could be doable in this regard, would the gettext project be open to such changes?
I've got general ideas as for how to implement this change, but I'd like to be sure such changes would be OK before going on further.

Thanks in advance!

@whatyouhide
Copy link
Contributor

@Gladear what would be the difference between a "custom storage model" vs what #305 proposes?

@whatyouhide whatyouhide changed the title discussion: Using custom storage models Custom storage models Aug 9, 2024
@Gladear
Copy link
Contributor Author

Gladear commented Aug 9, 2024

The idea would be to be much more flexible, and allow users to do more.

  • Currently : the only choice is to use the compiled backend. I don't think there's a way to implement my own backend that would - as per my example - fetch data using :persistent_term or - as per the use case targeted by Add support for runtime translations #305 - a database.
  • With Add support for runtime translations #305 : you get two choices, either use a compiled backend, or define a repo and fetch translations at runtime. There's no in between.

What I would like is to make the backend of Gettext 100% customizable. The idea is still a bit fuzzy in my mind, but here's a little more concretely what I think could be done:

Currently, when you use Gettext, Gettext.Compiler compile the .po files right into your module, implementing the Gettext.Backend behaviour. This could be implemented this through a Gettext.Storage.Compiled storage model. In Gettext.Compile, instead of compiling .po files directly, Gettext.Storage.Compiled would be called to generate lgettext/5 and lngettext/7. @external_resources and __mix_recompile__?/0 would have to be moved there too to avoid being present in other storage implementations.

use Gettext,
  otp_app: :my_app,
  storage_model: Gettext.Storage.Compiled # that would be the default value, equivalent to current implementation

Then you could have different models.

Let's take the same example again with an implementation through :persistent_term.

use Gettext,
  otp_app: :my_app,
  storage_model: MyApp.GettextStorage.PersistentTerm

MyApp.GettextStorage.PersistentTerm would again generate two functions lgettext/5 and lngettext/7 that would fetch translations in :persistent_term (or in a database).

@Gladear
Copy link
Contributor Author

Gladear commented Aug 9, 2024

I created a prototype on a fork to better demonstrate this feature request. It's far from perfect implementation-wise, but works (more or less, but the errors come from the Storage.PersistentTerm module).
There are absolutely no changes to the current API, all tests pass correctly.

https://github.com/Gladear/gettext/commits/storage-model-prototype/

The last commits adds a sample application that can be run to show the working PersistentTerm storage.

# iex -S mix
iex(1)> StorageModelSample.hello
"No"

Whilst hello calls gettext("Hello, world!"),
and priv/gettext/en/LC_MESSAGES/default.po contains

msgid "Hello, world!"
msgstr "No"

The main interest of this change works fine too : modifying default.po doesn't require me to recompile the whole project, only to restart the application.

@whatyouhide
Copy link
Contributor

Okay, I took another read here and I think this can be squished together with the issue that #305 is solving: #280.

So, basically what these two issues are looking for is:

  • A way to disable the compilations of translations. That's slow and not dynamic, so doesn't work for some use cases.
  • A way to fetch translations through custom logic, be it fetching it from persistent term or from wherever.

@Gladear, I think what you would need for your custom implementation is just the list of PO files that Gettext knows of. With that, you could just implement your custom translation fetcher that fetches from persistent_term, and you could do so through the mechanism defined in #305—provided that you have away to avoid compiling stuff into the backend.

Is that correct? @bamorim @maennchen do you folks have thoughts?

@Gladear
Copy link
Contributor Author

Gladear commented Aug 16, 2024

This does work for my particular use case indeed, but I do think only allowing users to choose between either "compile" or "runtime" can be limiting.

One other issue/use case I thought of (that is fixed by custom storage models) is #380. Having a custom compiled backend would allow to make changes to the translations during compilation, and therefore ensure the HTML in translations is safe.

Thinking twice about it, there's nothing preventing the translation fetcher functions to be generated at compile time. Works for me!

@maennchen
Copy link
Member

maennchen commented Aug 16, 2024

@whatyouhide The reason why we're talking about #330 and the options for custom backends at the same time, is that the compilation right now is so extremely slow because of the compile time decpendency.

I believe we should look at the two issues separately.

If we got gettext to the point where a normal usage at runtime in a module would constitute an export depdendency instead of a compile time one, it would massively cut the compilation time of the project since usually only the backend module itself has to recompile.

This would make it feasible to recompile the gettext backend module at runtime as well and makes me question if we even still need custom backends or if those should just update the PO file and recompile the backend.

Supporting custom storage backends itself is honestly not a feature i desperately want for this library. It's literally called gettext after the file we're reading. I think for translations sourced from cloud translation providers or a database, it doesn't have to be in scope for this library.

@whatyouhide
Copy link
Contributor

That's a really good callout @maennchen, runtime translations can be implemented on top of this library right now. I will close this and continue the discussion in the other issue. Should we also close #305?

@Gladear
Copy link
Contributor Author

Gladear commented Aug 17, 2024

I'm sorry, I'm not sure I understand, I would like to avoid any confusion 😄

Do you mean that runtime translations should be implemented on top of gettext but in another library?

I like the idea, but I'm not sure how feasible that would be, or how much of gettext would be re-usable.

@whatyouhide
Copy link
Contributor

@Gladear pluralization or locale maybe can be reused from this library but yeah the rest is totally just a separate concern. But @maennchen is right, this is Gettext so if you're reading translations from somewhere else than a PO/MO/POT file... then you probably can remove the Gettext dependency and just use something else, since Gettext wouldn't really help in that case.

@whatyouhide whatyouhide reopened this Aug 18, 2024
@whatyouhide whatyouhide closed this as not planned Won't fix, can't repro, duplicate, stale Aug 18, 2024
@Gladear
Copy link
Contributor Author

Gladear commented Aug 18, 2024

Just a small remark: you both say gettext should remain gettext and only work with PO files, which I can understand. However, #391 introduced a very simple way to use any kind of backend.

If I understand correctly (I can't test right now, I don't have a computer with me), I just have to create my own backend that implements the Gettext.Backend behaviour, and use it in use Gettext, backend: MyCustomBackend. The doc does say "you have to use Gettext.Backend", but nothing is stopping me from doing it my way. Sooo, is that something that should be prevented, or is it a happy coincidence that this will now be possible?

@whatyouhide
Copy link
Contributor

If I understand correctly (I can't test right now, I don't have a computer with me), I just have to create my own backend that implements the Gettext.Backend behaviour, and use it in use Gettext, backend: MyCustomBackend.

Yeah this would work but it's a happy coincidence: what's the point of doing that? You get automatic extraction into POT files anyway so then you sort of needs Gettext to use those. 😄

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants