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 #5

Merged
merged 56 commits into from
Apr 16, 2024
Merged

merge #5

merged 56 commits into from
Apr 16, 2024

Conversation

ppodolsky
Copy link

No description provided.

Frando and others added 26 commits March 27, 2024 12:54
## Description

Based on #2085 

The downloader already has the feature to try multiple nodes for a
single hash or hashseq. With #2085 we expose the downloader over the RPC
protocol, by adding a `DownloadMode { Queued, Direct }` enum to the
`BlobDownloadRequest`.
This PR modifies the `BlobDownloadRequest` to include a list of provider
nodes for a hash instead of just a single node. For queued requests that
go to the downloader, the nodes are just passed to the downloader. For
direct requests, the nodes are tried in-order, until one succeeds.

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.
## Description

fix(iroh-bytes): do not log when a file can not be deleted...

because it is not there. this makes the delete op idempotent, and also
helps in case the file was not there in the first place.

Fixes #2142

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.
## Description

The order of labels when parsing a DNS query as pkarr name was reversed.
This means all queries for second-level subdomains under a pkarr pubkey
failed.

* First commit is the actual fix. 
* Second commit adds a test for various record name and type variations
* Third and forth commit improve the debug and info logging

## Notes & open questions

Was discovered by @rklaehn in #2188 and fixes the wrong behavior
observed there.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.
@ppodolsky ppodolsky merged commit da0ea4d into izihawa:main Apr 16, 2024
5 of 6 checks passed
ppodolsky added a commit that referenced this pull request Apr 20, 2024
* refactor: downloader

* test: add test for concurrent progress reporting

* chore: cleanup

* refactor: rename some things for clarity

* feat: limit concurrent dials per download

* chore: cleanup

* refactor: respect progress sender IDs to allow reusing the senders in subsequent operations

* chore: fmt & clippy

* cleanup: shared download progress

* feat: handle tags in the downloader

* fix: visibility

* fixup

* fixup

* chore: clippy

* fixup

* refactor: make export a seperate operation from download

* fixup after merge

* refactor: remove id mapping

* refactor: move BlobId up to the progress events

* fix tests

* cleanup

* fix: invariants

* fixes and cleanups

* fix: nodes_have should only add to queued downloads

* chore: fmt

* chore: clippy!

* fix: do not queue downloads with no providers

* feat: set more than one node on download requests (n0-computer#2128)

## Description

Based on n0-computer#2085 

The downloader already has the feature to try multiple nodes for a
single hash or hashseq. With n0-computer#2085 we expose the downloader over the RPC
protocol, by adding a `DownloadMode { Queued, Direct }` enum to the
`BlobDownloadRequest`.
This PR modifies the `BlobDownloadRequest` to include a list of provider
nodes for a hash instead of just a single node. For queued requests that
go to the downloader, the nodes are just passed to the downloader. For
direct requests, the nodes are tried in-order, until one succeeds.

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.

* fix: address review by divma

* fix: remove now obsolete into_send method from ProgressSender

* chore: remove unused code

* fix: use Display for DownloadKind

* fix: first round of changes after review

* docs: downloader next_step

* more review

* more review

* fix: only remove hashes from provider map if they are really not needed

* refactor: move TagSet into util

* refactor: store intents in request info

* cleanup

* refactor: no allocation in next_step and simpler code

* fix: test after merge

* refactor: better method

* fix(iroh-bytes): do not log redundant file delete error (n0-computer#2199)

## Description

fix(iroh-bytes): do not log when a file can not be deleted...

because it is not there. this makes the delete op idempotent, and also
helps in case the file was not there in the first place.

Fixes n0-computer#2142

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.

* fix(iroh-dns-server): fix bug in pkarr name parsing (n0-computer#2200)

## Description

The order of labels when parsing a DNS query as pkarr name was reversed.
This means all queries for second-level subdomains under a pkarr pubkey
failed.

* First commit is the actual fix. 
* Second commit adds a test for various record name and type variations
* Third and forth commit improve the debug and info logging

## Notes & open questions

Was discovered by @rklaehn in n0-computer#2188 and fixes the wrong behavior
observed there.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.

---------

Co-authored-by: Franz Heinzmann (Frando) <frando@unbiskant.org>
Co-authored-by: Rüdiger Klaehn <rklaehn@protonmail.com>
ppodolsky added a commit that referenced this pull request Apr 20, 2024
* refactor: downloader

* test: add test for concurrent progress reporting

* chore: cleanup

* refactor: rename some things for clarity

* feat: limit concurrent dials per download

* chore: cleanup

* refactor: respect progress sender IDs to allow reusing the senders in subsequent operations

* chore: fmt & clippy

* cleanup: shared download progress

* feat: handle tags in the downloader

* fix: visibility

* fixup

* fixup

* chore: clippy

* fixup

* refactor: make export a seperate operation from download

* fixup after merge

* refactor: remove id mapping

* refactor: move BlobId up to the progress events

* fix tests

* cleanup

* fix: invariants

* fixes and cleanups

* fix: nodes_have should only add to queued downloads

* chore: fmt

* chore: clippy!

* fix: do not queue downloads with no providers

* feat: set more than one node on download requests (n0-computer#2128)

## Description

Based on n0-computer#2085 

The downloader already has the feature to try multiple nodes for a
single hash or hashseq. With n0-computer#2085 we expose the downloader over the RPC
protocol, by adding a `DownloadMode { Queued, Direct }` enum to the
`BlobDownloadRequest`.
This PR modifies the `BlobDownloadRequest` to include a list of provider
nodes for a hash instead of just a single node. For queued requests that
go to the downloader, the nodes are just passed to the downloader. For
direct requests, the nodes are tried in-order, until one succeeds.

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.

* fix: address review by divma

* fix: remove now obsolete into_send method from ProgressSender

* chore: remove unused code

* fix: use Display for DownloadKind

* fix: first round of changes after review

* docs: downloader next_step

* more review

* more review

* fix: only remove hashes from provider map if they are really not needed

* refactor: move TagSet into util

* refactor: store intents in request info

* cleanup

* refactor: no allocation in next_step and simpler code

* fix: test after merge

* refactor: better method

* fix(iroh-bytes): do not log redundant file delete error (n0-computer#2199)

## Description

fix(iroh-bytes): do not log when a file can not be deleted...

because it is not there. this makes the delete op idempotent, and also
helps in case the file was not there in the first place.

Fixes n0-computer#2142

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.

* fix(iroh-dns-server): fix bug in pkarr name parsing (n0-computer#2200)

## Description

The order of labels when parsing a DNS query as pkarr name was reversed.
This means all queries for second-level subdomains under a pkarr pubkey
failed.

* First commit is the actual fix. 
* Second commit adds a test for various record name and type variations
* Third and forth commit improve the debug and info logging

## Notes & open questions

Was discovered by @rklaehn in n0-computer#2188 and fixes the wrong behavior
observed there.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.

---------

Co-authored-by: Franz Heinzmann (Frando) <frando@unbiskant.org>
Co-authored-by: Rüdiger Klaehn <rklaehn@protonmail.com>
ppodolsky added a commit that referenced this pull request Apr 20, 2024
* refactor: downloader

* test: add test for concurrent progress reporting

* chore: cleanup

* refactor: rename some things for clarity

* feat: limit concurrent dials per download

* chore: cleanup

* refactor: respect progress sender IDs to allow reusing the senders in subsequent operations

* chore: fmt & clippy

* cleanup: shared download progress

* feat: handle tags in the downloader

* fix: visibility

* fixup

* fixup

* chore: clippy

* fixup

* refactor: make export a seperate operation from download

* fixup after merge

* refactor: remove id mapping

* refactor: move BlobId up to the progress events

* fix tests

* cleanup

* fix: invariants

* fixes and cleanups

* fix: nodes_have should only add to queued downloads

* chore: fmt

* chore: clippy!

* fix: do not queue downloads with no providers

* feat: set more than one node on download requests (n0-computer#2128)

## Description

Based on n0-computer#2085 

The downloader already has the feature to try multiple nodes for a
single hash or hashseq. With n0-computer#2085 we expose the downloader over the RPC
protocol, by adding a `DownloadMode { Queued, Direct }` enum to the
`BlobDownloadRequest`.
This PR modifies the `BlobDownloadRequest` to include a list of provider
nodes for a hash instead of just a single node. For queued requests that
go to the downloader, the nodes are just passed to the downloader. For
direct requests, the nodes are tried in-order, until one succeeds.

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.

* fix: address review by divma

* fix: remove now obsolete into_send method from ProgressSender

* chore: remove unused code

* fix: use Display for DownloadKind

* fix: first round of changes after review

* docs: downloader next_step

* more review

* more review

* fix: only remove hashes from provider map if they are really not needed

* refactor: move TagSet into util

* refactor: store intents in request info

* cleanup

* refactor: no allocation in next_step and simpler code

* fix: test after merge

* refactor: better method

* fix(iroh-bytes): do not log redundant file delete error (n0-computer#2199)

## Description

fix(iroh-bytes): do not log when a file can not be deleted...

because it is not there. this makes the delete op idempotent, and also
helps in case the file was not there in the first place.

Fixes n0-computer#2142

## Notes & open questions

<!-- Any notes, remarks or open questions you have to make about the PR.
-->

## Change checklist

- [ ] Self-review.
- [ ] Documentation updates if relevant.
- [ ] Tests if relevant.

* fix(iroh-dns-server): fix bug in pkarr name parsing (n0-computer#2200)

## Description

The order of labels when parsing a DNS query as pkarr name was reversed.
This means all queries for second-level subdomains under a pkarr pubkey
failed.

* First commit is the actual fix. 
* Second commit adds a test for various record name and type variations
* Third and forth commit improve the debug and info logging

## Notes & open questions

Was discovered by @rklaehn in n0-computer#2188 and fixes the wrong behavior
observed there.

## Change checklist

- [x] Self-review.
- [x] Documentation updates if relevant.
- [x] Tests if relevant.

---------

Co-authored-by: Franz Heinzmann (Frando) <frando@unbiskant.org>
Co-authored-by: Rüdiger Klaehn <rklaehn@protonmail.com>
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.

3 participants