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

Organize scripts (remove Python, use NodeJS 20+ directly instead of github-script, move to TypeScript) #411

Open
ohadschn opened this issue Nov 7, 2023 · 2 comments
Labels

Comments

@ohadschn
Copy link
Collaborator

ohadschn commented Nov 7, 2023

UPDATE It looks like the newest https://github.com/actions/github-script uses NodeJS 20, so dropping it might be less critical, though probably still preferable - mostly because it doesn't really offer much, see our availability monitoring script where we do just fine without it...

OpenAI for python just reached GA (1.0.x) this week: (https://pypi.org/project/openai/1.1.1/#history)
Breaking changes / migration guide: openai/openai-python#631

However, I suggest that rather than invest the time in upgrading, we take this opportunity to encapsulate the python logic into the NodeJS (auto-pr) script. There isn't really a need for 2 separate scripts, and most of our devs know JS better than python. Also less CI work (only need to install node runtime/dependencies, etc). It also looks like the NPM package is more mature (its 1.X.Y version has been release 2 years ago: https://www.npmjs.com/package/openai?activeTab=versions).

While we're at it, I think we can also move off github-script to regular NodeJS. The former is limited to Node16 (missing some useful features) and has an annoying package adding experience (custom require method). Its main advantage can be easily obtained by using https://octokit.github.io/rest.js/v20 directly. The rest of the context is provided in environment variables anyway by the GH actions runtime.

Finally, it it's always better to use TypeScript than JavaScript.

@ohadschn ohadschn changed the title Upgrade to OpenAI 1.0.x Upgrade to OpenAI 1.0.x + organize scripts (remove Python, use NodeJS 20 directly instead of github-script) Nov 8, 2023
@ohadschn ohadschn changed the title Upgrade to OpenAI 1.0.x + organize scripts (remove Python, use NodeJS 20 directly instead of github-script) Organize scripts (remove Python, use NodeJS 20 directly instead of github-script) Nov 8, 2023
@ohadschn ohadschn changed the title Organize scripts (remove Python, use NodeJS 20 directly instead of github-script) Organize scripts (remove Python, use NodeJS 20+ directly instead of github-script) Nov 8, 2023
@ohadschn ohadschn added the devops label Nov 8, 2023
@4tal
Copy link
Collaborator

4tal commented Nov 9, 2023

+1 for NodeJs

@4tal 4tal moved this to Ready For Work in Link For Israel Nov 9, 2023
@ohadschn ohadschn changed the title Organize scripts (remove Python, use NodeJS 20+ directly instead of github-script) Organize scripts (remove Python, use NodeJS 20+ directly instead of github-script, move to TypeScript) Nov 19, 2023
@ohadschn
Copy link
Collaborator Author

@4tal following our WhatsApp discussion, the "move to typescript" part applies to the link availability monitor script as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Ready For Work
Development

No branches or pull requests

2 participants