Closed rbenza closed 1 year ago
Airsaid
commented
1 year ago I don't have that problem on my end, it looks like the corresponding class is not downloaded from the maven repository.
I ran it and found that the OpenAI Icon image is a bit too big, so I will replace it.
Airsaid
commented
1 year ago I made some changes, can you see if it's OK now?
rbenza
commented
1 year ago I made some changes, can you see if it's OK now?
Yes, looks fine to me. Fixed my issue, was loading the incomplete JAR file (without external dependencies) instead of the complete ZIP file inside build\distributions
Also fixed incorrect request body formatting. I've added a system role for ChatGPT to increase the translation quality. But I do experience this issue now:
Caused by: java.net.SocketTimeoutException: Read timed out
at java.base/java.net.SocketInputStream.socketRead0(Native Method)
at java.base/java.net.SocketInputStream.socketRead(SocketInputStream.java:115)
at java.base/java.net.SocketInputStream.read(SocketInputStream.java:168)
at java.base/java.net.SocketInputStream.read(SocketInputStream.java:140)
at java.base/sun.security.ssl.SSLSocketInputRecord.read(SSLSocketInputRecord.java:478)
at java.base/sun.security.ssl.SSLSocketInputRecord.readHeader(SSLSocketInputRecord.java:472)
at java.base/sun.security.ssl.SSLSocketInputRecord.bytesInCompletePacket(SSLSocketInputRecord.java:70)
at java.base/sun.security.ssl.SSLSocketImpl.readApplicationRecord(SSLSocketImpl.java:1454)
at java.base/sun.security.ssl.SSLSocketImpl$AppInputStream.read(SSLSocketImpl.java:1065)
at java.base/java.io.BufferedInputStream.fill(BufferedInputStream.java:252)
at java.base/java.io.BufferedInputStream.read1(BufferedInputStream.java:292)
at java.base/java.io.BufferedInputStream.read(BufferedInputStream.java:351)
at java.base/sun.net.www.http.HttpClient.parseHTTPHeader(HttpClient.java:754)
at java.base/sun.net.www.http.HttpClient.parseHTTP(HttpClient.java:689)
at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream0(HttpURLConnection.java:1615)
at java.base/sun.net.www.protocol.http.HttpURLConnection.getInputStream(HttpURLConnection.java:1520)
at java.base/sun.net.www.protocol.http.HttpURLConnection.getHeaderField(HttpURLConnection.java:3103)
at java.base/java.net.URLConnection.getHeaderFieldLong(URLConnection.java:636)
at java.base/java.net.URLConnection.getContentLengthLong(URLConnection.java:508)
at java.base/sun.net.www.protocol.https.HttpsURLConnectionImpl.getContentLengthLong(HttpsURLConnectionImpl.java:398)
at com.intellij.util.io.HttpRequests$RequestImpl.doReadBytes(HttpRequests.java:435)
... 31 more@Airsaid is it correct to use connection.setConnectTimeout(0); on the requestBuilder to prevent this issue? Does your library wait to move to the next string value until the server response is received? Regardless on how long it takes?
Airsaid
commented
1 year ago It's great, I ran the latest version and it works fine.
Using the infinite timeout I don't think is very good, it does block the currently translated string.
The timeout issue is usually a problem with the user's network or translation API service, and there is nothing we can do on the plugin side to fix it. Maybe we could set a slightly longer timeout instead of an infinite timeout?
Also, what is the approximate increase in accuracy after adding system roles to ChatGPT? Because currently, every translation will carry the system role command, which will significantly increase the token usage.
rbenza
commented
1 year ago It's great, I ran the latest version and it works fine.
Using the infinite timeout I don't think is very good, it does block the currently translated string.
The timeout issue is usually a problem with the user's network or translation API service, and there is nothing we can do on the plugin side to fix it. Maybe we could set a slightly longer timeout instead of an infinite timeout?
Also, what is the approximate increase in accuracy after adding system roles to ChatGPT? Because currently, every translation will carry the system role command, which will significantly increase the token usage.
Think the ChatGPT system role is important to get the best result. It's around 100 tokens so only 0.0002 dollar per translated string. If you translate 500 strings this would be 0.10 dollar. That's acceptable to me. Maybe we can shorten it a bit.
However I still often get timeout errors. Can not be my connection since i'm on high end 1gbit/s connection. One thing we should add is at least return all the currently translated strings, before the error happens.. Since now everytime an error occurs all the previously translated strings are not being returned. So you waste money each time a error happens since OpenAI is a paid service.
Another thing that would be great is make the notification dynamic. So 'translating to Italian... (string 2 of 423)' that keeps updating so you know the current progress.
Airsaid
commented
1 year ago It's great, I ran the latest version and it works fine. Using the infinite timeout I don't think is very good, it does block the currently translated string. The timeout issue is usually a problem with the user's network or translation API service, and there is nothing we can do on the plugin side to fix it. Maybe we could set a slightly longer timeout instead of an infinite timeout? Also, what is the approximate increase in accuracy after adding system roles to ChatGPT? Because currently, every translation will carry the system role command, which will significantly increase the token usage.
Think the ChatGPT system role is important to get the best result. It's around 100 tokens so only 0.0002 dollar per translated string. If you translate 500 strings this would be 0.10 dollar. That's acceptable to me. Maybe we can shorten it a bit.
However I still often get timeout errors. Can not be my connection since i'm on high end 1gbit/s connection. One thing we should add is at least return all the currently translated strings, before the error happens.. Since now everytime an error occurs all the previously translated strings are not being returned. So you waste money each time a error happens since OpenAI is a paid service.
Another thing that would be great is make the notification dynamic. So 'translating to Italian... (string 2 of 423)' that keeps updating so you know the current progress.
I've seen the last few commits shorten the command.
The problem of interruption after an error is indeed a big optimization point, I will optimize the logic of this piece in the next few days.
Also about optimizing the notification content, this is also a good suggestion, I should be able to optimize it in the next version.
I don't think this PR is much of a problem anymore. Is there anything else to optimize on your end? If not, I think it can be merged.
rbenza
commented
1 year ago Yes, you can merge the PR. But I would only do a release if you fix the issue that the translated (and paid for) strings are returned when an error occurs.
Airsaid
commented
1 year ago I merged it, thanks again for your contribution!
@Airsaid I still get the error below when trying to translate a strings.xml file. I'm not fully sure if I need to add code to the
SettingsComponentclass. Maybe you can edit / provide feedback?