swarm: push tags integration - request flow #1347

acud · 2019-04-23T10:43:03Z

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

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

acud · 2019-04-23T11:23:39Z

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?

acud · 2019-04-26T02:13:25Z

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

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 · 2019-04-27T06:47:51Z

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 swarm/chunk: add tags #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

acud · 2019-04-27T08:47:56Z

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.

acud · 2019-04-27T08:56:39Z

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

swarm/chunk: address (some) PR comments swarm/chunk: add seen field swarm/chunk: change eta status call swarm/chunk: address pr comments swarm/chunk: address pr comments swarm/chunk: address pr comments swarm/chunk: remove ambiguity of total swarm/api: wip add chunk totals swarm/api: tag test-bed swarm/api, swarm/chunk: wip rudimentary implementation for tag increments swarm/api, swarm/storage: initial TestApiPut passes swarm/api: remove logline swarm: address PR comments cmd, swarm: fix compilation errors swarm/chunk: adjust test swarm/chunk: adjust tests swarm/chunk: pointers galore swarm: fix tests and compilation swarm/storage: change pyramid chunker append signature swarm/api: make api tags private swarm/api: add test for larger content cmd, swarm: add test for http upload swarm: remove deprecated and unused code, add swarm hash to DoneSplit signature swarm/chunk: add marshalling for address field swarm/api: add handle get tag handler, add tags for post raw cmd/swarm: reinstate recursive flag swarm/api: cleanup api test swarm/api: cleanup api test swarm/api/client: add client tag check to multipart upload swarm/api/client: add more tag assertions to tests swarm/api/http: test for correct tag size estimate according to Content-Length header swarm/api/http: document test swarm/api/http: uncomment test swarm/storage: address pr comments swarm/api: remove silly comment

acud · 2019-05-01T01:53:22Z

@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

swarm/api/api_test.go

zelig

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

swarm/chunk/tags.go

…ain content-length

…ntent length and number of chunks in a trie

…assertion

…pendency solved

zelig

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

swarm/api: integrate tags to count chunks being split and stored swarm/api/http: integrate tags in middleware for HTTP `POST` calls and assert chunks being calculated and counted correctly swarm: remove deprecated and unused code, add swarm hash to DoneSplit signature, remove calls to the api client from the http package

acud added the push-sync label Apr 23, 2019

acud requested a review from zelig April 23, 2019 10:43

acud self-assigned this Apr 23, 2019

acud mentioned this pull request Apr 23, 2019

[WIP] Push tags #1318

Closed

janos reviewed Apr 23, 2019

View reviewed changes

cmd/swarm/swarm-smoke/upload_and_sync.go Outdated Show resolved Hide resolved

swarm/fuse/swarmfs_test.go Outdated Show resolved Hide resolved

acud added the in progress label Apr 23, 2019

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

acud commented Apr 26, 2019

View reviewed changes

zelig reviewed Apr 27, 2019

View reviewed changes

acud changed the base branch from add-tags-struct to swarm-rather-stable April 30, 2019 04:13

acud force-pushed the tagz-count branch 2 times, most recently from 8dc9f72 to 4f92cac Compare April 30, 2019 04:32

acud force-pushed the tagz-count branch from 4f92cac to cd10ff2 Compare April 30, 2019 04:51

zelig mentioned this pull request Apr 30, 2019

push syncing - meta + timeline #1028

Closed

20 tasks

acud added 2 commits April 30, 2019 16:27

swarm: fix compile errors

b0a891d

swarm: fix tests, address pr comments

466387e

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

acud commented May 1, 2019

View reviewed changes

swarm/api/api_test.go Outdated Show resolved Hide resolved

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

4061571

zelig suggested changes May 1, 2019

View reviewed changes

acud added 2 commits May 2, 2019 00:00

swarm: WIP address PR comments

8734b3b

swarm/chunk: add Tags.All() method

e4b472f

acud added 2 commits May 2, 2019 09:04

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

d01da18

swarm: rename chunk states as per janos recommendation

44469a2

zelig reviewed May 2, 2019

View reviewed changes

swarm/chunk/tags.go Outdated Show resolved Hide resolved

swarm/chunk/tags.go Outdated Show resolved Hide resolved

acud added 9 commits May 2, 2019 13:57

swarm/chunk: pr comments

a6bc167

swarm/api/http: add function to calculate number of chunks for a cert…

212ebf2

…ain content-length

swarm: fix api test, fix chunk estimate test, move to int64 due to co…

d262054

…ntent length and number of chunks in a trie

swarm/api: mend flaking test, create test for ecnrypted chunk number …

000dd3c

…assertion

swarm/api/http: inline needed functionality

6cdf4d8

swarm/api/http: remove api client dependency

4d0be7c

swarm/api/http: remove client dependency

c68107f

swarm/api/client: fix header name ref to http package after cyclic de…

683d0c2

…pendency solved

swarm/api/http: minor cosmetics

8c71b9f

zelig reviewed May 4, 2019

View reviewed changes

zelig approved these changes May 4, 2019

View reviewed changes

acud force-pushed the tagz-count branch from 1b6bb64 to f8b2373 Compare May 5, 2019 17:04

swarm: address pr comments

f628f74

acud force-pushed the tagz-count branch from f8b2373 to f628f74 Compare May 5, 2019 17:24

acud removed the in progress label May 5, 2019

acud merged commit c966633 into swarm-rather-stable May 5, 2019

acud deleted the tagz-count branch May 5, 2019 18:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

swarm: push tags integration - request flow #1347

swarm: push tags integration - request flow #1347

acud commented Apr 23, 2019 •

edited

Loading

janos left a comment

acud commented Apr 23, 2019 •

edited

Loading

acud Apr 26, 2019

zelig commented Apr 27, 2019

acud commented Apr 27, 2019

acud commented Apr 27, 2019

acud commented May 1, 2019

zelig left a comment

zelig left a comment

swarm: push tags integration - request flow #1347

swarm: push tags integration - request flow #1347

Conversation

acud commented Apr 23, 2019 • edited Loading

janos left a comment

Choose a reason for hiding this comment

acud commented Apr 23, 2019 • edited Loading

acud Apr 26, 2019

Choose a reason for hiding this comment

zelig commented Apr 27, 2019

acud commented Apr 27, 2019

acud commented Apr 27, 2019

acud commented May 1, 2019

zelig left a comment

Choose a reason for hiding this comment

zelig left a comment

Choose a reason for hiding this comment

acud commented Apr 23, 2019 •

edited

Loading

acud commented Apr 23, 2019 •

edited

Loading