Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

feat: Add Alibaba machine translate service #10237

Merged
merged 5 commits into from
Dec 4, 2023

Conversation

hussainshaikh12
Copy link
Contributor

Proposed changes

Fixes #7659

Checklist

  • Lint and unit tests pass locally with my changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added documentation to describe my feature.
  • I have squashed my commits into logic units.
  • I have described the changes in the commit messages.

Other information

  1. Tested the feature locally and it works perfectly.
    Screenshot from 2023-10-20 00-05-48

@nijel nijel added this to the 5.2 milestone Oct 23, 2023
@codecov
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

Attention: Patch coverage is 95.74468% with 2 lines in your changes missing coverage. Please review.

Project coverage is 90.86%. Comparing base (c0b226d) to head (f0b8928).
Report is 4697 commits behind head on main.

Files with missing lines Patch % Lines
weblate/machinery/alibaba.py 92.85% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #10237   +/-   ##
=======================================
  Coverage   90.85%   90.86%           
=======================================
  Files         546      547    +1     
  Lines       56694    56741   +47     
  Branches     9038     9040    +2     
=======================================
+ Hits        51512    51557   +45     
- Misses       3596     3597    +1     
- Partials     1586     1587    +1     
Files with missing lines Coverage Δ
weblate/machinery/forms.py 87.17% <100.00%> (+0.69%) ⬆️
weblate/machinery/models.py 100.00% <ø> (ø)
weblate/machinery/tests.py 99.85% <100.00%> (+<0.01%) ⬆️
weblate/machinery/alibaba.py 92.85% <92.85%> (ø)

@hussainshaikh12
Copy link
Contributor Author

Hey @nijel @orangesunny do i need to do anything else here ?

Copy link
Member

@nijel nijel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides above feedback, can you please add tests for this backend?

Comment on lines 31 to 32
Alibaba
---
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Alibaba
---
Alibaba
-------

requirements-optional.txt Outdated Show resolved Hide resolved
@hussainshaikh12
Copy link
Contributor Author

hussainshaikh12 commented Nov 2, 2023

Besides above feedback, can you please add tests for this backend?

How should i write tests ? as they dont have a specific url to fetch and i am using their sdk. It is similar to aws where they are using a stubber but i guess that same stubber wont work here as it is aws specific

@nijel
Copy link
Member

nijel commented Nov 6, 2023

You can use generic Python mock instead, or you can use responses to mock HTTP for the library.

@nijel nijel modified the milestones: 5.2, 5.3 Nov 10, 2023
@nijel
Copy link
Member

nijel commented Nov 27, 2023

As for the tests, the library seems to be built on top of requests, so you can use existing mocking by responses, just define mocked responses in mock_response method of the test. You can use IBM service as an example:

class IBMTranslationTest(BaseMachineTranslationTest):
MACHINE_CLS = IBMTranslation
EXPECTED_LEN = 1
ENGLISH = "en"
SUPPORTED = "zh-TW"
CONFIGURATION = {
"url": "https://api.region.language-translator.watson.cloud.ibm.com/"
"instances/id",
"key": "",
}
def mock_empty(self):
raise SkipTest("Not tested")
def mock_error(self):
raise SkipTest("Not tested")
def mock_response(self):
responses.add(
responses.GET,
"https://api.region.language-translator.watson.cloud.ibm.com/"
"instances/id/v3/languages?version=2018-05-01",
json={"languages": [{"language": "en"}, {"language": "zh-TW"}]},
)
responses.add(
responses.POST,
"https://api.region.language-translator.watson.cloud.ibm.com/"
"instances/id/v3/translate?version=2018-05-01",
json={
"translations": [{"translation": "window"}],
"word_count": 1,
"character_count": 6,
},
)

@hussainshaikh12
Copy link
Contributor Author

As for the tests, the library seems to be built on top of requests, so you can use existing mocking by responses, just define mocked responses in mock_response method of the test. You can use IBM service as an example:

class IBMTranslationTest(BaseMachineTranslationTest):
MACHINE_CLS = IBMTranslation
EXPECTED_LEN = 1
ENGLISH = "en"
SUPPORTED = "zh-TW"
CONFIGURATION = {
"url": "https://api.region.language-translator.watson.cloud.ibm.com/"
"instances/id",
"key": "",
}
def mock_empty(self):
raise SkipTest("Not tested")
def mock_error(self):
raise SkipTest("Not tested")
def mock_response(self):
responses.add(
responses.GET,
"https://api.region.language-translator.watson.cloud.ibm.com/"
"instances/id/v3/languages?version=2018-05-01",
json={"languages": [{"language": "en"}, {"language": "zh-TW"}]},
)
responses.add(
responses.POST,
"https://api.region.language-translator.watson.cloud.ibm.com/"
"instances/id/v3/translate?version=2018-05-01",
json={
"translations": [{"translation": "window"}],
"word_count": 1,
"character_count": 6,
},
)

Tried this, but it still makes the request and gives InvalidAccessKeyId.NotFound error

======================================================================
ERROR: test_translate (weblate.machinery.tests.AlibabaTranslationTest)

Traceback (most recent call last):
File "/home/hussain/Desktop/open/weblate/weblate/machinery/base.py", line 409, in _translate
result = [
File "/home/hussain/Desktop/open/weblate/weblate/machinery/base.py", line 409, in
result = [
File "/home/hussain/Desktop/open/weblate/weblate/machinery/alibaba.py", line 275, in download_translations
response = self.client.do_action_with_exception(request)
File "/home/hussain/Desktop/open/venv/lib/python3.10/site-packages/aliyunsdkcore/client.py", line 501, in do_action_with_exception
raise exception
aliyunsdkcore.acs_exception.exceptions.ServerException: HTTP Status: 404 Error:InvalidAccessKeyId.NotFound Specified access key is not found. RequestID: D4E7621B-5052-55EE-B947-E4CDD671FDE3

@nijel
Copy link
Member

nijel commented Dec 1, 2023

Ah, they are using a vendored requests instead of a dependency. It should be possible to patch that as well using responses, but there is really no documentation on that, see getsentry/responses#188

Looking at the responses code, the following might work (not tested at all):

class AlibabaTranslationTest(BaseMachineTranslationTest): 
    ....
    mock = responses.RequestsMock(target="aliyunsdkcore.vendored.requests.adapters.HTTPAdapter.send")
    
     def mock_response(self):
         self.mock.add(...)

     @mock.activate
     def test_translate(self, **kwargs):
         super().test_translate(**kwargs)

@hussainshaikh12
Copy link
Contributor Author

Followed the approach from GoogleV3Translation and used the patch.object to override the call for the do_action_with_exception method and this works finally.

@nijel nijel merged commit 2b30f7a into WeblateOrg:main Dec 4, 2023
24 checks passed
@nijel
Copy link
Member

nijel commented Dec 4, 2023

Merged, thanks for your contribution!

nijel added a commit that referenced this pull request Dec 4, 2023
@nijel nijel self-assigned this Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Alibaba Translate machine translate service
2 participants