Updated deprecated dependency request-promise-native #576
Conversation
providers/store/webhookDeltaStore.js
Outdated
| const response = await callFetch(options) | ||
| if (response.status !== 200) | ||
| this.logger.info(`Failure Firing webhook failed: ${response.status} ${response.statusText}`) |
There was a problem hiding this comment.
This function could not be tested as I was not able to find the correct flow to hit this function.
There was a problem hiding this comment.
setup to test the webhookDeltaStore locally:
-
in .env file, set the following env variables:
- CRAWLER_STORE_PROVIDER=cdDispatch+cd(file)+webhook
- CRAWLER_WEBHOOK_URL="http://service:4000/webhook"
- CRAWLER_WEBHOOK_TOKEN="Bearer github_token_here"
- WEBHOOK_CRAWLER_SECRET="Bearer github_token_here"
-
trigger a harvest in the crawler. Upon completion of harvest the highlighted code will be executed.
-
To ensure the message has been correctly transmitted to service, can also set a break point at handleCrawlerCall in service. Verify that the response status is set to 200.
There was a problem hiding this comment.
Thank you @qtomlinson for taking out time to help me with testing this portion of the code change. I followed these steps and I am able to test the remaining portion as well.
providers/fetch/podFetch.js
Outdated
| async _getSourceArchive(dir, url, podspec) { | ||
| const archive = path.join(dir.name, `${podspec.name}-${podspec.version}.archive`) | ||
| const output = path.join(dir.name, `${podspec.name}-${podspec.version}`) | ||
| const response = await callFetch({ url: url, responseType: 'stream' }) | ||
| return new Promise((resolve, reject) => { | ||
| request({ url: url, json: false, encoding: null }).pipe( | ||
| response.pipe( |
There was a problem hiding this comment.
This function could not be tested as I was not able to find the pod component without github url detail which would have triggered this function.
There was a problem hiding this comment.
The above code path can be hit using one of these coordinates. Please also consider adding one of these to integration test.
There was a problem hiding this comment.
Thank you @qtomlinson for the list of coordinates. It was very helpful and this functionality has been covered in the testing now. We can add one of the version of pod/cocoapods/-/xcbeautify coordinate to our integration test cases as the size of this particular component is least out of all the coordinates that I tested.
providers/fetch/cratesioFetch.js
Outdated
| const response = await callFetch({ | ||
| url: `https://crates.io${version.dl_path}`, | ||
| responseType: 'stream', | ||
| headers: { | ||
| 'User-Agent': 'clearlydefined.io crawler ([email protected])', | ||
| Accept: 'text/html' | ||
| } | ||
| }) |
There was a problem hiding this comment.
The header 'Accept: text/html' is added to make sure we get the crate package in the stream response. If we do not add this header in the request the response is coming out to be a json containing the download URL for that crate. request-promise-native was redirecting and getting crate package as response by default but while using axios we have to specify this header to directly get the crate package. @lumaxis I request your opinion on this approach.
There was a problem hiding this comment.
@ajhenry Your feedback is much appreciated!
There was a problem hiding this comment.
This makes sense to me as we're essentially mimicking what a web crawler would request 👍
There was a problem hiding this comment.
Thanks for the confirmation!
Qt/pull fetch
Also added more unit test
Support uri in options
The crawler code has been updated to replace the deprecated dependency, request-promise-native, with axios. The following tasks have been performed:
Future scope -
This PR is related to the issue #555