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

Market: fix rest responses #765

Merged
merged 19 commits into from
Dec 15, 2020
Merged

Conversation

tworec
Copy link
Contributor

@tworec tworec commented Nov 4, 2020

It breaks compatibility of Market API, so not to be merged before Alpha.3.
fixes #450 resolves golemfactory/golem-internal#204
depends on: golemfactory/ya-client#59

Contains changes from #850 and #869

@tworec tworec requested review from nieznanysprawiciel and a team November 4, 2020 15:12
@tworec tworec self-assigned this Nov 4, 2020
@tworec tworec requested a review from prekucki November 4, 2020 15:12
Wiezzel
Wiezzel previously approved these changes Nov 5, 2020
Copy link

@Wiezzel Wiezzel left a comment

Choose a reason for hiding this comment

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

Look OK, but please merge the changes in ya-client to master and update commit hash in Cargo.toml.

@@ -153,7 +153,7 @@ async fn approve_agreement(
.approve_agreement(id, &agreement_id, timeout)
.await
.log_err()
.map(|_| HttpResponse::NoContent().finish())
.map(|status| HttpResponse::Ok().json(status.to_string()))
Copy link
Contributor Author

@tworec tworec Nov 16, 2020

Choose a reason for hiding this comment

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

should be 410 Gone as suggested by @prekucki in case of Cancelled agreement

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed here golemfactory/ya-client#70

@tworec tworec mentioned this pull request Dec 9, 2020
2 tasks
Copy link
Contributor

@jiivan jiivan left a comment

Choose a reason for hiding this comment

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

lgtm

@tworec tworec changed the base branch from master to event-api/master December 11, 2020 15:11
Cargo.toml Outdated Show resolved Hide resolved
@tworec tworec requested a review from a team December 14, 2020 23:07
@tworec tworec merged commit 9b2708c into event-api/master Dec 15, 2020
@tworec tworec deleted the market/fix-rest-responses branch December 15, 2020 09:56
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.

Fix unsubscribe's result type
5 participants