-
Notifications
You must be signed in to change notification settings - Fork 4
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
NOISSUE - Add functionality to sdk-js #2
Conversation
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
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.
also consider adding validation before making requests
const options = { | ||
method: "post", | ||
maxBodyLength: Infinity, | ||
url: `${this.bootstraps_url}/things/${this.bootstrapsEndpoint}`, |
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.
consider using the url object https://developer.mozilla.org/en-US/docs/Web/API/URL/URL
|
||
|
||
class Errors { | ||
HandleError(error_dict, status_code) { |
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.
where is error_dict defined
401: "Missing or invalid access token provided.", | ||
}, | ||
login: { | ||
404: "A non-existent entity request.", |
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.
404: "A non-existent entity request.", |
let subtopic = ""; | ||
|
||
if (chan_name_parts.length == 2) { | ||
subtopic = chan_name_parts[1].replace(".", "/", -1); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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.
also consider adding validation before making requests
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.
An additional note on error handling, I think the js way is to throw error rather than return error, also add an informative error message simillar to go SDK
"Content-Type": this.content_type, | ||
Authorization: `Bearer ${token}`, | ||
}, | ||
data: JSON.stringify(config), |
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.
axios converts js objects to json so no need for JSON.stringify() look at other simillar cases
}; | ||
return axios.request(options) | ||
.then((_response) => { | ||
return "DELETED"; |
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.
perhaps return the status code
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
Signed-off-by: Musilah <[email protected]>
I've done all these changes in the pull request #4. |
What does this do?
Add functioning code to JS SDK.
Which issue(s) does this PR fix/relate to?
No issue
List any changes that modify/break current functionality
Have you included tests for your changes?
Yes. Manually tested.
Did you document any new/modified functionality?
No
Notes