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

[FEATURE] Rework and centralize api client operation #189

Merged
merged 5 commits into from
May 5, 2023
Merged

Conversation

NarkNiro
Copy link
Member

@NarkNiro NarkNiro commented May 5, 2023

Closes #188

@calien666 calien666 added this to the v3.x Release milestone May 5, 2023
sbuerk added 4 commits May 5, 2023 15:55
The `DEEPLMOCKAPISERVER` is only a really rudimentary
api implementation and do not simulate "full" api in
any way. That means, that not all source/target languages
are supported like for production api.

Therefore, one test is skipped now when `DEEPLMOCKAPISERVER`
is in use because of not supporting `EN` as target language.
An additionally test is added to test from `EN` to `DE` in
anny case.

Furthermore, when mock server is used the supported content
and and translated strings are used for test expectations.
The `DEEPLMOCKAPISERVER` is only a really rudimentary
api implementation and do not simulate "full" api in
any way. That means, that not all source/target languages
are supported like for production api.

Therefore, when mock server is used, the supported content
and and translated strings are used for test expectations.
The `DEEPLMOCKAPISERVER` is only a really rudimentary
api implementation and do not simulate "full" api in
any way. That means, that not all source/target languages
are supported like for production api.

Therefore, when mock server is used, the supported content
and and translated strings are used for test expectations.

[TASK] Donate fine grained method to retrieve api url parts

`\WebVision\WvDeepltranslate\Configuration` transports the
required configuration settings through the system, which
is a good approach.

This change donates more fine grained read methods to this
class, to access api url parts. This is a preparation for
an upcoming change.
In a couple of places, `https` has been hardcoded enforced
as request scheme, ignoring the scheme of the configured
api url.

This may seem to be a "security" enforcement, but these
kind of overrides are not very user friendly and hidden
magic is always a pain in the ass for debugging issues.

This change now re-uses the configured api url scheme
when building api requests. One test is adopted to do
the same.

The donated methods on the `Configuration` class to
access api url parts are used for this.
@NarkNiro NarkNiro merged commit d7c22eb into master May 5, 2023
@NarkNiro NarkNiro deleted the feature/188 branch May 5, 2023 14:18
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.

[FEATURE] Improve Client Handling
3 participants