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.

Sign up for GitHub

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

Jump to bottom

refactor!: no generic BeeResponse returned from Bee class #342

Merged
merged 2 commits into from
Jun 8, 2021
Merged

refactor!: no generic BeeResponse returned from Bee class #342

merged 2 commits into from
Jun 8, 2021

Conversation

AuHau
Copy link
Contributor

@AuHau AuHau commented Jun 3, 2021

No description provided.

@AuHau AuHau requested a review from a team June 3, 2021 13:19
@AuHau AuHau requested a review from nugaon as a code owner June 3, 2021 13:19
nugaon
nugaon reviewed Jun 7, 2021
View reviewed changes
Copy link
Member

@nugaon nugaon left a comment •
edited
Loading

Choose a reason for hiding this comment

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

IMO It should be fixed in the Bee, do we have a ticket for this there?
and I also second @agazso's suggestion.

@AuHau
Copy link
Contributor Author

AuHau commented Jun 7, 2021

IMO It should be fixed in the Bee, do we have a ticket for this there?

Not sure what do you want to fix? I don't think there is anything wrong on Bee's side here. They use this structure to communicate for example errors as well. It is maybe a bit inconsistent as some endpoints return the data directly, while other endpoints follow this BeeResponse structure...

@nugaon
Copy link
Member

nugaon commented Jun 7, 2021

Not sure what do you want to fix? I don't think there is anything wrong on Bee's side here. They use this structure to communicate for example errors as well. It is maybe a bit inconsistent as some endpoints return the data directly, while other endpoints follow this BeeResponse structure...

It seems the pinning returns with 200 status code despite the operation failed, that I think is not correct. Otherwise the axios should throw an error I guess and the safeAxios would throw this error.

@AuHau
Copy link
Contributor Author

AuHau commented Jun 7, 2021

It seems the pinning returns with 200 status code despite the operation failed, that I think is not correct. Otherwise the axios should throw an error I guess and the safeAxios would throw this error.

Wait, where do you see this? I am not aware of Bee returning 200 for failed operation...

@AuHau AuHau mentioned this pull request Jun 8, 2021
Closed
@AuHau AuHau merged commit d2a65ee into master Jun 8, 2021
@AuHau AuHau deleted the refactor/no-bee-response branch June 8, 2021 13:45
@bee-worker bee-worker mentioned this pull request Jun 8, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nugaon nugaon nugaon left review comments

@agazso agazso agazso approved these changes

Assignees
No one assigned
Labels
type:pull-request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AuHau @nugaon @agazso