Closed bertfrees closed 3 months ago
bertfrees
commented
6 months ago @marisademeglio I would like the next version (January release) to make use of the properties API for at least the properties for which we are currently making use of the "Text-to-speech configuration file" options. Does that sound feasible?
marisademeglio
commented
6 months ago @bertfrees No I still don't think it's feasible to incorporate this for 1.2.7, scheduled Jan 15-19. It would push us back to beta and we are already at RC3. Is there an advantage for end users to setting the properties this way?
bertfrees
commented
6 months ago For the GUI user there will be (almost) no difference. This is just a matter of keeping up with changes in the engine. Every release of the engine needs to be incorporated in a new GUI version, and the ability to set properties through the TTS config file was removed in the latest engine. I thought this would be a rather trivial thing to do in the GUI. I don't feel like reverting it: I want to move forward, and reverting would take a lot of time that I prefer to put into updating the GUI code myself (if you are too busy with the installer).
Why "push us back to beta"?
marisademeglio
commented
6 months ago We would definitely want to do a new round of testing with a change like this. There were many issues related to the tts config file, which we worked through, and I don't want to introduce a new round of bugs by changing it all at the last minute. My understanding, based on what you told me, was that both the config file and API endpoints would be supported, so we would not have to change the way the config works for this release. If that's not the case then we need more time to test.
NPavie
commented
4 months ago Some notes on the endpoint while trying a bit some things :
/voices endpoint (see 880a65b93b22b13ce40df5cc8e8c1f7db484a774 ), this means we need to also reload the voices list from the backend (and if we keep the same logical process, keep the properties in saved settings on connection validation. bertfrees
commented
4 months ago confirmed that end point is not persistent, so we need to reload the settings saved in the app settings file into the engine when it has finished started.
I think it would make sense to pass the saved settings at launch time (through environment variables, system properties or the pipeline.properties file) instead of using the /admin/properties API.
the endpoint is cors protected if i try to update the properties directly from the ui
This end point shouldn't behave any different from the other ones w.r.t. CORS. Are we using the org.daisy.pipeline.ws.cors setting?
NPavie
commented
4 months ago This end point shouldn't behave any different from the other ones w.r.t. CORS. Are we using the org.daisy.pipeline.ws.cors setting?
I see this in the launcher -Dorg.daisy.pipeline.ws.cors=true
Oh i think there is something else happening here.
The error i get in the UI is :
Access to fetch at 'http://127.0.0.1:49152/ws/admin/properties/org.daisy.pipeline.tts.azure.region' from origin 'http://localhost:4927' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: It does not have HTTP ok status.
Looking more precisely in the network console, it seems that the webview-side fetch API is first doing an "OPTIONS" request as part of a CORS-preflight evaluation for the PUT method; and the pipeline answer a 405 code. (The node fetch side does not make this, and this seems to happen only for the put method, fetch with post method does not make the OPTIONS request)
NPavie
commented
3 months ago should be ok from 3f2ba99e2dfc78b098e57302ca7f1814a129ba17 (might need some more tests to ensure everything is ok)
marisademeglio
commented
3 months ago It seems to be working fine for me
Get available properties (including descriptions):
Set a property: