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

fix: add InlineDNSLink flag to PublicGateways config #9328

Merged
merged 8 commits into from
Oct 11, 2022
Merged

fix: add InlineDNSLink flag to PublicGateways config #9328

merged 8 commits into from
Oct 11, 2022

Conversation

hacdias
Copy link
Member

@hacdias hacdias commented Oct 5, 2022 •
edited
Loading

Fixes #9243.

@hacdias hacdias marked this pull request as ready for review October 5, 2022 11:53
@hacdias hacdias requested a review from lidel as a code owner October 5, 2022 11:53
@hacdias hacdias force-pushed the fix/9243 branch 2 times, most recently from f1ea600 to eed6de2 Compare October 7, 2022 07:50
@hacdias hacdias self-assigned this Oct 10, 2022
@hacdias hacdias requested a review from Jorropo October 10, 2022 09:08
hacdias
hacdias commented Oct 10, 2022
View reviewed changes
config/gateway.go Outdated Show resolved Hide resolved
hacdias
hacdias commented Oct 10, 2022
View reviewed changes
docs/config.md Show resolved Hide resolved
@hacdias
Copy link
Member Author

hacdias commented Oct 11, 2022

@lidel per our conversation yesterday, I updated this PR to use Flag instead of bool.

lidel added 2 commits October 11, 2022 14:10
@lidel
docs: inlining DNSLink on subdomains
362fb39 
additional explanations, as this is very low level and nuanced feature
@lidel
refactor: UseInlinedDNSLink → InlineDNSLink
d1fdd76 
since this is a Flag and not bool, we can have shorter name
@lidel lidel changed the title fix: add UseInlinedDNSLink flag to configuration fix: add InlineDNSLink flag to PublicGateways config Oct 11, 2022
@hacdias
Copy link
Member Author

hacdias commented Oct 11, 2022 •
edited
Loading

@lidel the tests are now passing. It seems they were stuck in the Web UI, but I restarted them. Thanks for the docs updates and clarifications.

lidel
lidel approved these changes Oct 11, 2022
View reviewed changes
Copy link
Member

@lidel lidel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thank you @hacdias, lgtm.

@cypt4 fyi this will ship in Kubo 0.17. Brave will be able to enable this in config via:

$ ipfs config --json Gateway.PublicGateways '{
  "localhost": {
    "UseSubdomains": true,
    "InlineDNSLink": true,
    "Paths": ["/ipfs", "/ipns", "/api"]
  }
}'

@lidel lidel merged commit 4291d6b into master Oct 11, 2022
@lidel lidel deleted the fix/9243 branch October 11, 2022 13:46
@cypt4 cypt4 mentioned this pull request Nov 29, 2022
Merged
25 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lidel lidel lidel approved these changes

@Jorropo Jorropo Awaiting requested review from Jorropo

Assignees

@hacdias hacdias

Labels
None yet
Projects
No open projects
IPFS Shipyard Team
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Gateway: add UseInlinedDNSLink flag to config
2 participants
@hacdias @lidel