Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

gotta get green #1354

Merged
merged 6 commits into from
May 15, 2018
Merged

gotta get green #1354

merged 6 commits into from
May 15, 2018

Conversation

daviddias
Copy link
Member

No description provided.

@ghost ghost assigned daviddias May 14, 2018
@ghost ghost added the status/in-progress In progress label May 14, 2018
@daviddias
Copy link
Member Author

The DNS issue was just that the new gateways didn't have CORS enabled. From IRC:

21:36 <@daviddias> wait wat, how can we be using a dns module on npm which is a DNS server? https://www.npmjs.com/package/dns
21:38 <@victorbjelkholm> daviddias: eh? Where you see that? This is the dns resolve: https://github.com/ipfs/js-ipfs/blob/master/src/core/runtime/dns-browser.js#L11
21:38 <@victorbjelkholm> using fetch (go-ipfs http call)
21:38 <@daviddias> dns-resolve in the browser is fine
21:38 <@daviddias> only the node.js version
21:38 <@victorbjelkholm> hm, no? It's the webworker version that is broken no?
21:39 <@victorbjelkholm> (granted, I spent 1 minute looking at it) but I think it was the fetch call in webworkers who got the 403 response
21:39 <@daviddias> browser runs fine locally 
21:39 <@daviddias> nodej.js fails with timeout
21:39 <@daviddias> https://github.com/ipfs/js-ipfs/blob/master/src/core/runtime/dns-nodejs.js
21:39 <@daviddias> let me try webworker
21:39 <@victorbjelkholm> daviddias: isn't using the nodejs core modules dns rather than the npm modules dns?
21:39 <@daviddias> victorbjelkholm: it is
21:39 <@victorbjelkholm> the error alanshaw saw was not a timeout but 403 error
21:39 <@daviddias> I saw that incorrectly 
21:40 <@victorbjelkholm> daviddias: https://ci.ipfs.team/blue/organizations/jenkins/IPFS%2Fjs-ipfs/detail/master/103/tests
21:41 <@victorbjelkholm> ugh, annoying we don't see webworker/browser tests there...
21:41 <@daviddias> so
21:42 <@daviddias> did something change in jenkins?
21:42 <alanshaw> the dns tests seem to work locally for me
21:42 <alanshaw> only on jenkins is error
21:42 <alanshaw> https://ci.ipfs.team/blue/rest/organizations/jenkins/pipelines/IPFS/pipelines/js-ipfs/branches/fix%252Fdns/runs/2/nodes/20/steps/158/log/?start=0
21:42 <@daviddias> we got this
21:43 <@daviddias> the new IPFS gateways were deployd with a new config
21:43 <@daviddias> without the CORS being star
21:43 <@daviddias> fortunately, I have a lgierth here to fix it in a snap
21:43 <alanshaw> omg awesome
21:43 <@daviddias> lgierth: here is the 3 commands to get it right https://github.com/ipfs/js-ipfs-api/#cors

@lgierth is on top of it. We will spin CI once he finishes that.

@daviddias
Copy link
Member Author

It should be good now, the only thing missing is npm acknowledging that a new version has been published. Already published 0.65.7 and 0.65.8 and it doesn't show up.

image

alanshaw
alanshaw previously approved these changes May 15, 2018
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

LGTM. I see 0.65.8 on npm now too.

@daviddias daviddias merged commit 83d7c48 into master May 15, 2018
@ghost ghost removed the status/in-progress In progress label May 15, 2018
@daviddias daviddias deleted the gotta-get-green branch May 15, 2018 16:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants