Skip to content
This repository has been archived by the owner on Aug 22, 2018. It is now read-only.

Exposes x-watson-learning-opt-out request query parameter #35

Merged
merged 5 commits into from Jul 15, 2016

Conversation

nacho4d
Copy link
Collaborator

@nacho4d nacho4d commented Jul 6, 2016

I exposed this parameter since it could be a requirement for some clients

@@ -102,6 +102,7 @@ - (NSString *)getStartMessage{
[inputParameters setValue:self.interimResults forKey:@"interim_results"];
[inputParameters setValue:self.continuous forKey:@"continuous"];
[inputParameters setValue:self.inactivityTimeout forKey:@"inactivity_timeout"];
[inputParameters setValue:self.learningOptOut forKey:@"x-watson-learning-opt-out"];
Copy link
Collaborator

Choose a reason for hiding this comment

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

This parameter should be set into the WebSocket header, not here.

Copy link
Collaborator Author

@nacho4d nacho4d Jul 6, 2016

Choose a reason for hiding this comment

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

@mihui I was looking at the docs and they say query parameter:
http://www.ibm.com/watson/developercloud/doc/text-to-speech/websockets.shtml Am I looking at the correct place?

I checked in the demo app https://speech-to-text-demo.mybluemix.net/ in index.js and found that "X-Watson-Learning-Opt-Out" was supposed to go in the query parameter too but is actually not being set. It is a TODO. So I am confused. How can I check the value was correctly set?

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is TTS and it should be the same as STT. And yes, it says it is query parameter, so it should be like /v1/recognize?x-watson-learning-opt-out=true or /v1/synthesize?x-watson-learning-opt-out=true, not in the start message and should be before that.

But on Data Collection sections, it says "If you do not want to share your data, set the header parameter X-Watson-Learning-Opt-Out to true for each request. Data is collected for any request that omits this header.", see URLs below:

http://www.ibm.com/watson/developercloud/speech-to-text/api/v1/#data_collection
http://www.ibm.com/watson/developercloud/text-to-speech/api/v1/#data_collection
http://www.ibm.com/watson/developercloud/doc/getting_started/gs-logging.shtml

You can also review 4b20393 for STT and TTS.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see. You already did it right?! thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@nacho4d not all, will commit the TTS part.

Copy link
Collaborator

Choose a reason for hiding this comment

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

4b20393, 079e6c0, and c8e79bb are for TTS and STT, and there are more changes in pull request #33

@germanattanasio
Copy link
Owner

@nacho4d and @mihui you now have write access to this repo. I think you can both work together and don't need me to do the merge.

Great power comes with great responsibility
uncle_ben

@nacho4d
Copy link
Collaborator Author

nacho4d commented Jul 6, 2016

Thank you @germanattanasio !
Unless there is something urgent I personally will try to put any changes into PRs and take a couple of days of calm before merging stuff ;-]

@mihui
Copy link
Collaborator

mihui commented Jul 7, 2016

Thanks @germanattanasio , appreciated.

@nacho4d
Copy link
Collaborator Author

nacho4d commented Jul 9, 2016

@mihui I have updated this. I hope it is ok now :]


@property (readonly) NSString *token;
@property (copy, nonatomic) void (^tokenGenerator) (void (^tokenHandler)(NSString *token));

- (void) invalidateToken;
- (void) requestToken: (void(^)(AuthConfiguration *config)) completionHandler;
- (NSDictionary*) createRequestHeaders;
- (NSDictionary*) createRequestHeadersWithXWatsonLearningOptOut;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I prefer to keep only one "createRequestHeaders" method because it is only SDK-internal use, and change value through config. If the developers want to change the SDK code, they could do it, we just keep the code simple.

Thanks.

Copy link
Collaborator Author

@nacho4d nacho4d Jul 10, 2016

Choose a reason for hiding this comment

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

I see. What do you think about having a private header "AuthConfiguration+Internal.h" ? It should look something like this:

// AuthConfiguration+Internal.h
#import AuthConfiguration.h
@interface AuthConfiguration(Internal)
- (NSDictionary*) createRequestHeaders;
- (NSDictionary*) createRequestHeadersWithXWatsonLearningOptOut;
... other internal methods to be used only by the sdk...
@end

This way we can separate internal and internal stuff. In this case createRequestHeadersWithXWatsonLearningOptOut is used in STT and TTS request. IMO is better to have it as a method instead of doing it separately twice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It is a good idea to separate them, changing configuration is good too, either way is OK.

@nacho4d
Copy link
Collaborator Author

nacho4d commented Jul 14, 2016

@mihui I have the code. I hope it is ok now :]

@nacho4d
Copy link
Collaborator Author

nacho4d commented Jul 15, 2016

I am merging this since I have to ship something including this by this week.

@nacho4d nacho4d merged commit dca9625 into germanattanasio:master Jul 15, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants