-
Notifications
You must be signed in to change notification settings - Fork 11
Move API key in request header and change body to json format (#89) #90
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| require_once __DIR__ . '/../vendor/autoload.php'; | ||
|
|
||
| use Scn\DeeplApiConnector\DeeplClientFactory; | ||
|
|
||
| $apiKey = 'your-api-key'; | ||
|
|
||
| $usage = DeeplClientFactory::create($apiKey) | ||
| ->getUsage(); | ||
|
|
||
| var_dump($usage); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| require_once __DIR__ . '/../vendor/autoload.php'; | ||
|
|
||
| use Scn\DeeplApiConnector\DeeplClientFactory; | ||
| use Scn\DeeplApiConnector\Model\FileSubmissionInterface; | ||
| use Scn\DeeplApiConnector\Model\FileTranslationConfig; | ||
|
|
||
| $apiKey = 'your-api-key'; | ||
|
|
||
| $deepl = DeeplClientFactory::create($apiKey); | ||
|
|
||
| $fileTranslationConfig = new FileTranslationConfig( | ||
| 'Das ist ein Test', | ||
| 'testfile.txt', | ||
| 'EN', | ||
| 'DE', | ||
| ); | ||
| /** | ||
| * 1. We request a new translation with an FileTranslationConfig | ||
| */ | ||
| /** @var FileSubmissionInterface $fileSubmission */ | ||
| $fileSubmission = $deepl->translateFile($fileTranslationConfig); | ||
|
|
||
| /** | ||
| * Result look like a FileSubmission instance | ||
| * | ||
| * class Scn\DeeplApiConnector\Model\FileSubmission#22 (2) { | ||
| * private string $documentId => "<DOCUMENT_ID>" | ||
| * private string $documentKey => "<DOCUMENT_KEY>" | ||
| * } | ||
| */ | ||
| var_dump($fileSubmission); | ||
|
|
||
| /** | ||
| * We can simulate this by using our own instance with valid values: | ||
| * $fileSubmission = (new FileSubmission()) | ||
| * ->setDocumentId('<DOCUMENT_ID>') | ||
| * ->setDocumentKey('<DOCUMENT_KEY>'); | ||
| */ | ||
|
|
||
|
|
||
| /** 2. We request in a queue logic the translation status with the Submission instance **/ | ||
| sleep(15); | ||
| $translationStatus = $deepl->getFileTranslationStatus($fileSubmission); | ||
|
|
||
| /** | ||
| * Result look like a FileTranslationStatus instance | ||
| * | ||
| * if the 'status' property value is 'done' we can get the fileTranslation | ||
| * | ||
| * class Scn\DeeplApiConnector\Model\FileTranslationStatus#43 (4) { | ||
| * private string $documentId => "<DOCUMENT_ID>" | ||
| * private string $status => "translating" | ||
| * ..... | ||
| * } | ||
| */ | ||
| var_dump($translationStatus); | ||
|
|
||
|
|
||
| /** 3. We request in a queue logic the translation status with the Submission instance **/ | ||
| $response = $deepl->getFileTranslation($fileSubmission); | ||
|
|
||
| /** | ||
| * Result look like a FileTranslation instance | ||
| * | ||
| * class Scn\DeeplApiConnector\Model\FileTranslation#26 (1) { | ||
| * private string $content => "This is a test" | ||
| * } | ||
| */ | ||
| var_dump($response); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly nullable if you don't have one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it even make sense constructing the object without having an apikey? I don't think so...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One option for example is to throw an Exception('No API key given'), but the DeepL API cannot be used without apikey, whether you use the free or the Pro version makes no difference.
What solution is prefered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only reason to make the api-key nullable + default-null here (and as the last parameter in the list) is backward compatibility.
If the api-key is not nullable, then this change is not compatible and requires a new major version in case someone creates the client without using the factory.
However, since the code is no longer 100% backwards compatible even with an exception in the case of a missing api key, it will end up with a new major version anyway. And then we can do without it and not make the api key nullable.
Unless you find a way to implement the whole thing in a way that makes it backwards compatible. But since the effort is probably not worth it, I would personally be in favor of: non-nullable and a new major version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would agree with that "non-nullable and a new major version" to prevent surprises using this lib.