swarm: push tags integration - request flow #1347

justelad · Apr 23, 2019

this pr integrates the push tags into the low level components that actually do the bead-counting.
supersedes #1318

https://hackmd.io/9eWxJ_MJS8i04onWg49UBA?both

coincidentally resolves #1364 and fixes #1350

janos · Apr 23, 2019

janos reviewed Apr 23, 2019

View changes

There are compilations errors in swarm/api/http/test_server.go and swarm/fuse/swarmfs_test.go.

cmd/swarm/swarm-smoke/upload_and_sync.go

swarm/fuse/swarmfs_test.go

justelad · Apr 23, 2019

I am aware of the linting and compilation errors. This is still WIP (but preliminary implementation is there).
As I've noted to @zelig, I have a bit of a problem with the original design, namely:

the Tags.Inc method is not used since we need to pass around the Tag pointer anyway to the pyramid chunker
As of such we don't really need this to be a sync.Map but just a normal map that points to Tag pointers
I don't think we should pass the whole sync.Map to the chunker in order to do the increments. @janos @zelig WDYT?
we can pass the pointer in the context but I don't think this is a nice solution. I think we should just pass the tag id in the context and propagate the concrete object wherever necessary

opinions?

justelad · Apr 26, 2019

justelad reviewed Apr 26, 2019

View changes

justelad · Apr 26, 2019

swarm/api/api.go

@@ -297,31 +300,6 @@ func (a *API) ResolveURI(ctx context.Context, uri *URI, credentials string) (sto
 	return addr, nil
 }

-// Put provides singleton manifest creation on top of FileStore store
-func (a *API) Put(ctx context.Context, content string, contentType string, toEncrypt bool) (k storage.Address, wait func(context.Context) error, err error) {


this method was only used by tests. i think we should not export test helpers on public APIs. hence the functionality was removed from the API struct and duplicated wherever necessary

zelig · Apr 27, 2019

zelig reviewed Apr 27, 2019

View changes

swarm/chunk/tags.go

swarm/storage/filestore.go

swarm/storage/hasherstore.go

swarm/storage/pyramid.go

swarm/api/http/server.go

swarm/chunk/tag.go

swarm/chunk/tags.go

swarm/storage/hasherstore.go

zelig · Apr 27, 2019

can we please work with incremental PRs? Again we cram eveything into this.
you said it is ready for review but all checks fail on travis
Comment exported functions and in general please explain what you are doing:

what is the unit for which a tag is defined
how do they get default names
what are we doing with reponse headers

My 2c:

First we should finish #1341, putting all related changes there.
This PR should be only about creating and passing around tags, tag and the context
and chunker and hasherstore calling increment
Following PR: integrate in the response

justelad · Apr 27, 2019

can we please work with incremental PRs? Again we cram eveything into this.

localstore integration was one PR, shed was one PR, new localstore was one PR (not integrated)

you said it is ready for review but all checks fail on travis

ready for review means i would have tagged it ready for review and removed the [wip]. i haven't but i told you it's coming along and you can give it a pass and see how its coming along.

First we should finish #1341, putting all related changes there.

There were two days between my last commit on that branch and the last review. Not sure what I was supposed to do in between.

justelad · Apr 27, 2019

Following PR: integrate in the response

while working on this i figured there's not much use to integrate this in the response, since we lose the ability to pipe the returned hash values from swarm up to other processes/machine readable format. that's why i changed the DoneSplit() signature to set an associated root hash once the splitting has been done. this at a later stage could be queried against bzz-tag:/<swarm hash> and there a UI or JSON would be returned as a result of the Accept header

justelad · May 1, 2019

@zelig this is ready for review.
This PR addresses: creation of tags, passing them around, increments on store, seen, split, including tests that assert this. the bzz-tag frontend endpoint stub has been removed and will be deferred to a later PR.

Still left TODO:

Return the tag uid with a response header for dapps to track tags starting with the chunking phase
Integrate to localstore with an index that tracks the tag vectors per chunk on the push index. I don't see a way around this since we have to increment a single tag with SENT/SYNCED statuses
Rename chunk.State modes to the same convention as in http.StatusNotFound
Create the bzz-tag endpoint that will allow queries to return either a UI or JSON with the progress bars

justelad · May 1, 2019

justelad reviewed May 1, 2019

View changes

swarm/api/api_test.go

zelig · May 1, 2019

zelig requested changes May 1, 2019

View changes

nice one , getting there:

STORED increment should be after Put returns
if tests fail, we need syncronisation, ie wait for correct numbers with timeout
checkTag should work on one Tag
test helpers should be DRY-ed
chunk count estimation should become exact chunk counting and tested

swarm/api/client/client.go

swarm/api/client/client_test.go

swarm/api/http/server.go

swarm/api/http/server_test.go

swarm/network_test.go

swarm/storage/hasherstore.go

swarm/testutil/tag.go

zelig · May 2, 2019

zelig reviewed May 2, 2019

View changes

swarm/chunk/tags.go

zelig · May 4, 2019

zelig reviewed May 4, 2019

View changes

lint. shows unnecessary conversion https://travis-ci.org/ethersphere/go-ethereum/jobs/527810385#L257

swarm/api/http/server.go

swarm/chunk/tag.go

swarm/api/http/middleware.go

swarm/chunk/tags_test.go

swarm/storage/filestore.go

zelig · May 4, 2019

zelig approved these changes May 4, 2019

View changes

justelad added the area:push-sync label Apr 23, 2019

justelad requested a review from zelig Apr 23, 2019

justelad self-assigned this Apr 23, 2019

justelad referenced this pull request Apr 23, 2019
Closed
[WIP] Push tags #1318

justelad added the in progress label Apr 23, 2019

justelad changed the title ~~push tags integration~~ [wip]push tags integration Apr 24, 2019

zelig added this to Backlog in Swarm via automation Apr 29, 2019

zelig moved this from Backlog to Warm in Swarm Apr 29, 2019

justelad changed the base branch from add-tags-struct to swarm-rather-stable Apr 30, 2019

justelad force-pushed the tagz-count branch 2 times, most recently from 8dc9f72 to 4f92cac Apr 30, 2019

justelad force-pushed the tagz-count branch from 4f92cac to cd10ff2 Apr 30, 2019

zelig referenced this pull request Apr 30, 2019
Open
push syncing - meta + timeline #1028
9 of 19 tasks complete

justelad added some commits Apr 30, 2019

swarm: fix compile errors

b0a891d

swarm: fix tests, address pr comments

Loading status checks…

466387e

justelad changed the title ~~[wip]push tags integration~~ swarm: push tags integration - request flow May 1, 2019

swarm: fix linter, move stored increment so that tests dont flake

Loading status checks…

4061571

justelad added some commits May 1, 2019

swarm: WIP address PR comments

Loading status checks…

8734b3b

swarm/chunk: add Tags.All() method

e4b472f

justelad added some commits May 2, 2019

swarm/api, swarm/testutil: mend to use Tags.All() when CheckTag runs

d01da18

swarm: rename chunk states as per janos recommendation

Loading status checks…

44469a2

swarm: address pr comments

Loading status checks…

0adfeb6

justelad added some commits May 4, 2019

swarm: minor refinements to prevent test hanging

Loading status checks…

b73f1b6

swarm: prevent test hanging on older version of go

Loading status checks…

853bedd

ethersphere/go-ethereum forked from ethereum/go-ethereum

Join GitHub today

swarm: push tags integration - request flow #1347

Conversation

justelad commented Apr 23, 2019 • edited

justelad added the area:push-sync label Apr 23, 2019

justelad requested a review from zelig Apr 23, 2019

justelad self-assigned this Apr 23, 2019

justelad referenced this pull request Apr 23, 2019

[WIP] Push tags #1318

janos reviewed Apr 23, 2019 View changes

janos left a comment

justelad added the in progress label Apr 23, 2019

This comment has been minimized.

justelad commented Apr 23, 2019 • edited

justelad changed the title push tags integration [wip]push tags integration Apr 24, 2019

justelad reviewed Apr 26, 2019 View changes

This comment has been minimized.

justelad Apr 26, 2019

zelig reviewed Apr 27, 2019 View changes

This comment has been minimized.

zelig commented Apr 27, 2019

This comment has been minimized.

justelad commented Apr 27, 2019

This comment has been minimized.

justelad commented Apr 27, 2019

zelig added this to Backlog in Swarm via automation Apr 29, 2019

zelig moved this from Backlog to Warm in Swarm Apr 29, 2019

justelad changed the base branch from add-tags-struct to swarm-rather-stable Apr 30, 2019

justelad force-pushed the tagz-count branch 2 times, most recently from 8dc9f72 to 4f92cac Apr 30, 2019

justelad force-pushed the tagz-count branch from 4f92cac to cd10ff2 Apr 30, 2019

zelig referenced this pull request Apr 30, 2019

push syncing - meta + timeline #1028

justelad added some commits Apr 30, 2019

This comment has been minimized.

justelad commented May 1, 2019

justelad changed the title [wip]push tags integration swarm: push tags integration - request flow May 1, 2019

justelad reviewed May 1, 2019 View changes

zelig requested changes May 1, 2019 View changes

zelig left a comment

justelad added some commits May 1, 2019

justelad added some commits May 2, 2019

zelig reviewed May 2, 2019 View changes

justelad added some commits May 2, 2019

zelig reviewed May 4, 2019 View changes

zelig left a comment

zelig approved these changes May 4, 2019 View changes

justelad added some commits May 4, 2019

justelad commented Apr 23, 2019 •

edited

janos reviewed Apr 23, 2019

View changes

justelad commented Apr 23, 2019 •

edited

justelad reviewed Apr 26, 2019

View changes

zelig reviewed Apr 27, 2019

View changes

justelad force-pushed the tagz-count branch 2 times, most recently from `8dc9f72` to `4f92cac` Apr 30, 2019

justelad force-pushed the tagz-count branch from `4f92cac` to `cd10ff2` Apr 30, 2019

justelad reviewed May 1, 2019

View changes

zelig requested changes May 1, 2019

View changes

zelig reviewed May 2, 2019

View changes

zelig reviewed May 4, 2019

View changes

zelig approved these changes May 4, 2019

View changes