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

Merge from concat -> aleph nodes #152

Merged
merged 12 commits into from Jan 11, 2017
Merged

Merge from concat -> aleph nodes #152

merged 12 commits into from Jan 11, 2017

Conversation

yusefnapora
Copy link
Contributor

@yusefnapora yusefnapora commented Jan 5, 2017

This is basically working, but there's a few missing pieces:

  • no support for "partially successful" merges. If an error is encountered after some number of successful statement / object ingestions, the merge promise will fail, and you don't get the counts
  • I'm not yet verifying that the multihash for data objects we receive matches their keys
  • need to test the signStatement / verifyStatement code

I need to look through this some more tomorrow to make sure I'm not forgetting anything; I can't see the forest for the trees at the moment :)

@parkan
Copy link
Contributor

parkan commented Jan 6, 2017

I'm assuming build failures here are related to what vyzo is working on right now?

@yusefnapora
Copy link
Contributor Author

pretty sure this was another case of go dependencies getting out of sync with our docker image. I rebuilt it and re-ran... one failed with "MAC invalid" (wtf?)

@parkan
Copy link
Contributor

parkan commented Jan 6, 2017

yeah I've seen the MAC invalid error a few times in this situation (every time?)

{hello: 'world'},
{foo: 'bar'},
{etc: 'and so on'}
{id: uuid.v4(), hello: 'world'},
Copy link
Contributor

Choose a reason for hiding this comment

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

Curious, was there a specific reason for doing this? Are the tests noticeably faster this way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to make sure that data from one test wouldn't "contaminate" the results for a subsequent test, since I'm asserting on e.g. the number of objects merged, and if I unthinkingly reuse the same {foo: "bar"} object in multiple tests, I might get the wrong result if the node already had it in the store from a previous test.

I tried doing a garbage collection pass on the datastore after each test, but you have to take the node offline first, and I was getting weird connection reset errors. So I figured I'd just make sure that all the data objects had a random component, so there's no chance of a duplicate object throwing things off.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, that makes sense

@parkan parkan merged commit 1b8aa89 into master Jan 11, 2017
@yusefnapora yusefnapora deleted the yn-aleph-merge branch January 13, 2017 17:02
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.

None yet

2 participants