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

Adapt to new Market Api 1.6.1 #775

Merged
merged 1 commit into from
Nov 9, 2020
Merged

Conversation

tworec
Copy link
Contributor

@tworec tworec commented Nov 9, 2020

Conform to changes in golemfactory/ya-client#52

@tworec tworec requested review from nieznanysprawiciel and a team November 9, 2020 12:04
@tworec tworec self-assigned this Nov 9, 2020
@tworec tworec added the t-mkt label Nov 9, 2020
@tworec tworec added this to In progress in Mkt & Prov (Yagna Core) via automation Nov 9, 2020
@tworec tworec added this to the Mkt 1.2 (intro to Event API) milestone Nov 9, 2020
@tworec tworec force-pushed the market/new-market-api-1.6.1 branch from a7e7f8e to ca3052b Compare November 9, 2020 12:06
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

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.

Looks fine, but you gotta merge golemfactory/ya-client#52 ;)

Ok(ProposalResponse::RejectProposal)
Ok(ProposalResponse::RejectProposal {
reason: Some(format!(
"proposal expired at: {} which is less than 5 min or more than 30 min from now",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"proposal expired at: {} which is less than 5 min or more than 30 min from now",
"Proposal expires at: {} which is less than 5 min or more than 30 min from now",

Ok(ProposalResponse::RejectProposal)
Ok(ProposalResponse::RejectProposal {
reason: Some(format!(
"available agreements limit: {} reached",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"available agreements limit: {} reached",
"Node busy. Reached Agreements limit: {}",

Ok(AgreementResponse::RejectAgreement)
Ok(AgreementResponse::RejectAgreement {
reason: Some(format!(
"available agreements limit: {} reached",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"available agreements limit: {} reached",
"Node busy. Reached Agreements limit: {}",

@@ -16,7 +16,10 @@ pub enum ProposalResponse {
offer: DemandOfferBase,
},
AcceptProposal,
RejectProposal,
#[display(fmt = "RejectProposal( reason: {:?})", reason)]
Copy link
Contributor

Choose a reason for hiding this comment

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

It will be awfull message with Some()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's cover it in other PR, as this is blocking others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reported as #776

@tworec tworec merged commit e0d0944 into master Nov 9, 2020
Mkt & Prov (Yagna Core) automation moved this from In progress to Done Nov 9, 2020
@tworec tworec deleted the market/new-market-api-1.6.1 branch November 9, 2020 13:40
@nieznanysprawiciel nieznanysprawiciel moved this from Done to Archive in Mkt & Prov (Yagna Core) Nov 12, 2020
@tworec tworec moved this from Archive to Done in Mkt & Prov (Yagna Core) Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants