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.

Sign up for GitHub

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

Jump to bottom

Provide root node immediately on add and pin add #6068

Merged
merged 13 commits into from
Mar 20, 2019
Merged

Provide root node immediately on add and pin add #6068

merged 13 commits into from
Mar 20, 2019

Conversation

michaelavila
Copy link
Contributor

@michaelavila michaelavila commented Mar 8, 2019 •
edited by eingenito
Loading

Provide root node immediately when ipfs add <cid> and ipfs pin add <cid>.

This PR introduces part of the new provider subsystem from the rest of the ongoing provider work so it's more extensive than it might otherwise have been. This seemed like a good way to merge some of that work and begin using it. And he work to get this PR mergeable will get folded back into #5870

Addresses: #6067

@michaelavila michaelavila added the status/WIP This a Work In Progress label Mar 8, 2019
@ghost ghost assigned michaelavila Mar 8, 2019
@ghost ghost added the status/in-progress In progress label Mar 8, 2019
@michaelavila
Provide root node immediately when add and pin add
a9b6534 
License: MIT
Signed-off-by: Michael Avila <davidmichaelavila@gmail.com>
@michaelavila michaelavila force-pushed the features/provide-roots-on-add-and-pin branch from 9301556 to a9b6534 Compare March 8, 2019 22:28
@michaelavila
Remove timeout from provide context
d786a69 
This is being removed because it appears that the provide announcements
go out regardless of the timeout.

License: MIT
Signed-off-by: Michael Avila <davidmichaelavila@gmail.com>
@michaelavila
Use offlineProvider when --offline
bfcea27 
License: MIT
Signed-off-by: Michael Avila <davidmichaelavila@gmail.com>
@michaelavila michaelavila force-pushed the features/provide-roots-on-add-and-pin branch 4 times, most recently from fdcf655 to dafc9de Compare March 9, 2019 01:08
@michaelavila
Refactor per code climate rules
dde397e 
License: MIT
Signed-off-by: Michael Avila <davidmichaelavila@gmail.com>
@michaelavila michaelavila force-pushed the features/provide-roots-on-add-and-pin branch from dafc9de to dde397e Compare March 11, 2019 16:55
@ghost ghost assigned eingenito Mar 14, 2019
Erik Ingenito added 2 commits March 14, 2019 16:54
Provider queue updates to address deadlocks
8337035 
License: MIT
Signed-off-by: Erik Ingenito <erik@carbonfive.com>
Provider tests
7d07347 
License: MIT
Signed-off-by: Erik Ingenito <erik@carbonfive.com>
@eingenito eingenito force-pushed the features/provide-roots-on-add-and-pin branch 2 times, most recently from a36a278 to 6cefbb6 Compare March 15, 2019 18:10
Cleanup, fix broken restart, and more tests.
5d4f3fb 
License: MIT
Signed-off-by: Erik Ingenito <erik@carbonfive.com>
@eingenito eingenito force-pushed the features/provide-roots-on-add-and-pin branch from 6cefbb6 to 5d4f3fb Compare March 15, 2019 18:23
Erik Ingenito added 2 commits March 15, 2019 14:19
Remove locking entirely
1595253 
License: MIT
Signed-off-by: Erik Ingenito <erik@carbonfive.com>
Gofmt
7fcafb6 
License: MIT
Signed-off-by: Erik Ingenito <erik@carbonfive.com>
@eingenito eingenito force-pushed the features/provide-roots-on-add-and-pin branch from 25474eb to 7fcafb6 Compare March 15, 2019 23:47
Make queue operation more clear
6c1eca9 
License: MIT
Signed-off-by: Erik Ingenito <erik@carbonfive.com>
@eingenito eingenito force-pushed the features/provide-roots-on-add-and-pin branch 2 times, most recently from 55f02ea to 2454dd3 Compare March 16, 2019 04:15
@eingenito eingenito changed the title [WIP] Provide root node immediately when add and pin add Provide root node immediately when add and pin add Mar 16, 2019
@eingenito eingenito force-pushed the features/provide-roots-on-add-and-pin branch from 2454dd3 to 47fb37f Compare March 16, 2019 04:19
@eingenito eingenito removed the status/WIP This a Work In Progress label Mar 16, 2019
@eingenito eingenito changed the title Provide root node immediately when add and pin add Provide root node immediately on add and pin add Mar 16, 2019
Erik Ingenito added 2 commits March 16, 2019 09:53
Don't do extra work in provider queue loop
906d2bd 
License: MIT
Signed-off-by: Erik Ingenito <erik@carbonfive.com>
Additional provider tests
09cc62e 
License: MIT
Signed-off-by: Erik Ingenito <erik@carbonfive.com>
@eingenito eingenito force-pushed the features/provide-roots-on-add-and-pin branch from 47fb37f to 09cc62e Compare March 16, 2019 16:54
Stebalien
Stebalien requested changes Mar 18, 2019
View reviewed changes
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

My only real comment is on querying the datastore. Otherwise, LGTM!

Thanks!

core/core.go Outdated Show resolved Hide resolved
provider/provider.go Outdated Show resolved Hide resolved
provider/queue.go Show resolved Hide resolved
provider/provider.go Show resolved Hide resolved
@Stebalien Stebalien mentioned this pull request Mar 19, 2019
Merged
@Stebalien
Copy link
Member

@michaelavila let's add some docs and then we can merge this and fix the dht query stuff later.

@michaelavila michaelavila force-pushed the features/provide-roots-on-add-and-pin branch from 13e417e to f1461d2 Compare March 20, 2019 21:15
@michaelavila
Copy link
Contributor Author

@Stebalien I believe all feedback was correctly accounted for. Please advise if not.

@michaelavila michaelavila force-pushed the features/provide-roots-on-add-and-pin branch from f1461d2 to e4badd3 Compare March 20, 2019 21:18
Stebalien
Stebalien requested changes Mar 20, 2019
View reviewed changes
provider/provider.go Outdated Show resolved Hide resolved
@michaelavila
Add comments; Check ctx.Err(); Move import
019af75 
License: MIT
Signed-off-by: Michael Avila <davidmichaelavila@gmail.com>
@michaelavila
Copy link
Contributor Author

@Stebalien Ok, thanks for looking. Updated as suggested. One note: my conditional is the opposite of yours, but I think yours was just a typo.

@michaelavila michaelavila force-pushed the features/provide-roots-on-add-and-pin branch from e4badd3 to 019af75 Compare March 20, 2019 22:16
Stebalien
Stebalien approved these changes Mar 20, 2019
View reviewed changes
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

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

Yes, my conditional was just a typo. Let's ship this!

@Stebalien Stebalien merged commit 2ec7bef into master Mar 20, 2019
@ghost ghost removed the status/in-progress In progress label Mar 20, 2019
@Stebalien
Copy link
Member

🎉

@Stebalien Stebalien deleted the features/provide-roots-on-add-and-pin branch March 20, 2019 22:52
@michaelavila michaelavila mentioned this pull request Mar 25, 2019
Merged
@michaelavila michaelavila mentioned this pull request May 14, 2019
Closed
@michaelavila michaelavila mentioned this pull request Jun 4, 2019
Closed
@michaelavila michaelavila mentioned this pull request Jul 30, 2019
Open
22 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Stebalien Stebalien Stebalien approved these changes

@Kubuxu Kubuxu Awaiting requested review from Kubuxu

Assignees

@michaelavila michaelavila

@eingenito eingenito

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@michaelavila @Stebalien @eingenito