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

Add the ability to override default IPNI endpoint #234

Merged
merged 1 commit into from
May 12, 2023

Conversation

masih
Copy link
Member

@masih masih commented May 12, 2023

lassie fetch was hard-coded to always use cid.contact. The changes introduce a new flag --ipniEndpoint to allow overriding the default endpoint used to discover content providers.

This feature is useful when debugging a local indexer instance or an explicit IPNI instance for content discovery/retrieval.

@masih masih requested review from rvagg and hannahhoward May 12, 2023 10:39
@codecov-commenter
Copy link

codecov-commenter commented May 12, 2023

Codecov Report

Merging #234 (ad7b5e8) into main (70641fe) will decrease coverage by 0.27%.
The diff coverage is 0.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #234      +/-   ##
==========================================
- Coverage   71.10%   70.83%   -0.27%     
==========================================
  Files          68       68              
  Lines        6063     6079      +16     
==========================================
- Hits         4311     4306       -5     
- Misses       1507     1528      +21     
  Partials      245      245              
Impacted Files Coverage Δ
cmd/lassie/fetch.go 0.00% <0.00%> (ø)

... and 7 files with indirect coverage changes

cmd/lassie/fetch.go Outdated Show resolved Hide resolved
`lassie fetch` was hard-coded to always use `cid.contact`. The changes
introduce a new flag `--ipniEndpoint` to allow overriding the default
endpoint used to discover content providers.

This feature is useful when debugging a local indexer instance or an
explicit IPNI instance for content discovery/retrieval.
@masih masih force-pushed the masih/ipni_endpoint_flag branch from fd6cdd4 to ad7b5e8 Compare May 12, 2023 16:54
@masih
Copy link
Member Author

masih commented May 12, 2023

Thank you for the review @kylehuntsman 🙌

@masih masih merged commit 9a431c5 into main May 12, 2023
@masih masih deleted the masih/ipni_endpoint_flag branch May 12, 2023 17:02
lassieOpts = append(lassieOpts, finderOpt)
} else if cctx.IsSet("ipniEndpoint") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add some unit tests for this new cmd flag?

Copy link
Member

Choose a reason for hiding this comment

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

we currently don't have tests that go down in to the cli, unfortunately, we mostly do integration tests through the daemon so most of the code in here isn't touched

Copy link
Member Author

@masih masih May 15, 2023

Choose a reason for hiding this comment

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

We can use a similar setup as go-car script test for lassie CLI

rvagg added a commit that referenced this pull request May 15, 2023
rvagg added a commit that referenced this pull request May 15, 2023
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.

5 participants