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 decentralized - provider counter draft proposal #418

Merged
merged 11 commits into from Jul 15, 2020

Conversation

nieznanysprawiciel
Copy link
Contributor

resolves: #363
resolves: #376

@nieznanysprawiciel nieznanysprawiciel marked this pull request as ready for review July 14, 2020 10:17
@nieznanysprawiciel nieznanysprawiciel requested a review from a team July 14, 2020 10:17
Copy link
Contributor

@tworec tworec left a comment

Choose a reason for hiding this comment

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

just started to read

Co-authored-by: Piotr Chromiec <tworec@golem.network>
core/market/decentralized/src/negotiation/common.rs Outdated Show resolved Hide resolved
core/market/decentralized/src/db/models/proposal.rs Outdated Show resolved Hide resolved
// Map model events to client RequestorEvent.
let db = self.db();
Ok(futures::stream::iter(events)
.then(|event| event.into_client_requestor_event(&db))
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

Suggested change
.then(|event| event.into_client_requestor_event(&db))
.then(|event| event.into_client_requestor_event(&self.common.db))

.filter_map(|event| event.ok())
.collect::<Vec<RequestorEvent>>())
fn db(&self) -> DbExecutor {
self.common.db.clone()
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed

@@ -29,6 +29,8 @@ pub enum RemoteProposalError {
Unsubscribed(SubscriptionId),
#[error("Offer/Demand [{0}] expired.")]
Expired(SubscriptionId),
#[error("Trying to counter not existing Proposal [{0}].")]
ProposalNotFound(ProposalId),
Copy link
Contributor

Choose a reason for hiding this comment

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

we are in the context of Proposal and this will always be used like RemoteProposalError::NotFound

Suggested change
ProposalNotFound(ProposalId),
NotFound(ProposalId),

.requestor_engine
.query_events(&demand_id, 1.2, Some(5))
.await?;
let proposal0 = requestor::expect_proposal(events)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

would be good to assert proposal0 props below eg. status Initial

tworec
tworec previously approved these changes Jul 15, 2020
tworec
tworec previously approved these changes Jul 15, 2020
@tworec tworec force-pushed the market-decentralized/provider-counter-draft-proposal-rb branch from 71e8ac7 to 8aecb3e Compare July 15, 2020 16:41
@tworec tworec force-pushed the market-decentralized/provider-counter-draft-proposal-rb branch 2 times, most recently from 71e8ac7 to b077496 Compare July 15, 2020 17:00
@tworec tworec merged commit dd7db9c into master Jul 15, 2020
@tworec tworec deleted the market-decentralized/provider-counter-draft-proposal-rb branch July 15, 2020 23:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Counter draft proposal - requestor Counter draft proposal - provider
2 participants