serpapi / serpapi-javascript

Scrape and parse search engine results using SerpApi.
https://serpapi.com
MIT License
45 stars 4 forks source link

fix: node-polyfills required to run in browsers #18

Closed subho57 closed 1 year ago

subho57 commented 1 year ago
Screenshot 2023-07-08 at 4 40 43 PM
subho57 commented 1 year ago

@zyc9012 I see the tests seem to be failing for missing api_key in the env. I've now added it to my repository secret. Can you trigger a workflow run again?

subhankar-trisetra commented 1 year ago

@zyc9012 It still says, api_key not found, are the tests and the env setup correctly? Am I doing anything wrong?

zyc9012 commented 1 year ago

@subho57 I'm wondering why the action is not run on your fork. Can you make it work so I don't have to trigger it on every push manually?

subho57 commented 1 year ago

@subho57 I'm wondering why the action is not run on your fork. Can you make it work so I don't have to trigger it on every push manually?

For that I think you need to alter your branch_protection rules probably, not sure though :-p. I've now set two secrets: API_KEY & SERPAPI_TEST_KEY. Hopefully another trigger should give us actual results. :-)

zyc9012 commented 1 year ago

@subho57 I meant to enable the actions here https://github.com/subho57/serpapi-javascript/actions.

For that I think you need to alter your branch_protection rules. I've now set two secrets: API_KEY & SERPAPI_TEST_KEY. Hopefully another trigger should give us actual results. :-)

It only works for your fork, not in this repo.


I checked the rules of the secrets:

Anyone with collaborator access to this repository can use these secrets and variables for actions. They are not passed to workflows that are triggered by a pull request from a fork.

So I think the tests won't pass for PRs created by non-collaborators at this time. We need to sort this out. For now, you can make it run on your fork, I'll check the results from there. Thank you.

subho57 commented 1 year ago

Makes sense. Got it! However I've already exhausted my free credits. I can have another account setup and alter the API_KEY. But then I'm afraid, even that won't last through the entire test suite. @zyc9012 Can you help me with that?

zyc9012 commented 1 year ago

@subho57 Let me add some credits to your account. Could you give me the email?

subho57 commented 1 year ago

@subho57 Let me add some credits to your account. Could you give me the email?

developer.subho57@gmail.com

zyc9012 commented 1 year ago

@subho57 All set, should be good to go.

subho57 commented 1 year ago

@subho57 All set, should be good to go.

Great! Thanks, let me try

zyc9012 commented 1 year ago

@subho57. Alternatively, you can rebase your code on this branch remove-pagination (Related PR https://github.com/serpapi/serpapi-javascript/pull/17), which significantly reduces the amount of credits consumed per run.

subho57 commented 1 year ago

@zyc9012 I did run the workflow, turns out you were correct, with my change, the package only works with Node >= 18 (AbortController :: Node >=15 & fetch :: Node >= 18), which is not something we want. I'll try to find a dependency free alternate approach for this. Here's the report: https://github.com/subho57/serpapi-javascript/actions/runs/5498553358

zyc9012 commented 1 year ago

@zyc9012 I did run the workflow, turns out you were correct, with my change, the package only works with Node >= 18, which is not something we want. I'll try to find a dependency free alternate approach for this.

Thanks a lot! Let me know if you need any help.

subho57 commented 1 year ago

Closing this PR for now, will up with a new one hopefully :-)