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

Release setup-python as an npm package #38

Closed
peter-evans opened this issue Nov 10, 2019 · 15 comments
Closed

Release setup-python as an npm package #38

peter-evans opened this issue Nov 10, 2019 · 15 comments

Comments

@peter-evans
Copy link

peter-evans commented Nov 10, 2019

I have a Javascript action that executes Python. It would be great if I could add @actions/setup-python as a dependency and call it from my Javascript action.

In order to make it work for now I converted parts of this action into a setupPython function that I've included with my action. Below is a simplified snippet of what I'm doing now. Full source code here.

async function run() {
  try {
    const src = __dirname + "/src";

    // Setup Python from the tool cache
    setupPython("3.8.0", "x64");

    // Install requirements
    await exec.exec("pip", [
      "install",
      "--requirement",
      `${src}/requirements.txt`
    ]);

    // Execute python script
    await exec.exec("python", [`${src}/create-pull-request.py`]);
  } catch (error) {
    core.setFailed(error.message);
  }
}

Here is a python-action template that also demonstrates what I'm doing in the snippet above.

Why call Python from a Javascript action?
The reason I ended up doing this and not writing the whole action in Javascript requires a bit of history and explanation. I wrote the action during GitHub Actions beta v1 when it was just container actions. When Actions v2 arrived the choices were Javascript or Docker container actions. I want the action to be multi-platform and runnable on any virtual machine, so I have no choice but to wrap it with a Javascript action as I'm doing, or convert the whole action to Javascript.

Even if Docker container actions worked on all platforms I think I would still choose to wrap Python in Javascript like this. The two issues I don't like about container actions are:

  • When using image: 'Dockerfile' it's slow because the image is built from scratch every time the workflow runs.
  • When using image: 'docker://my-namespace/my-image:1.0.0' it cannot be forked easily because the reference to the public Docker image remains. Being able to fork GitHub actions is important for security conscious users.
@kevinresol
Copy link

Or it should support triggering another action from a javascript action, e.g.

import * as actions from '@actions/actions'; // <-- this is new
import * as exec from '@actions/exec';
await actions.run('setup-python', {python-version: '2.x'});
await exec.exec('pip install ...');

@peter-evans
Copy link
Author

It's not particularly pretty, but I think I've found a way to include setup-python as a dependency and execute it.

Add the repository as a dependency and specify a version.

npm i --save actions/setup-python#v2.0.1

It will get added to package.json like this.

  "dependencies": {
    "setup-python": "github:actions/setup-python#v2.0.1"
  }

Add a script in package.json to build setup-python.

  "scripts": {
    "build-setup-python": "npm install --prefix node_modules/setup-python && npm run build --prefix node_modules/setup-python"
    ...
  }

Install and build (Add this to CI, too)

npm install
npm run build-setup-python

It should build and be usable as in this example:

import * as core from '@actions/core'
import * as setupPython from 'setup-python/lib/find-python'

async function run(): Promise<void> {
  try {
    const installed = await setupPython.findPythonVersion('3.x', 'x64')
    core.info(`Successfully setup ${installed.impl} (${installed.version})`)
  } catch (error) {
    core.setFailed(error.message)
  }
}

run()

@brcrista
Copy link
Contributor

@peter-evans right, npm supports installing straight from a GitHub repo like you've found.

I think if we were publishing as an NPM package, we would want to do more work around giving the action a proper API and versioning the NPM package around that. So, if the existing path that npm provides is working well, I think that's a better alternative than taking on the dev and maintenance cost of a full-on package.

@peter-evans
Copy link
Author

@brcrista It feels a bit hacky so I don't think it's a great solution long term. It would be nice if the logic existed in an actions/tookit module and this action was just a wrapper that called it.

For my own use case, I'm planning to move away from executing Python with actions/exec and convert to Typescript. So eventually this will be a non-issue for me, but others may still appreciate a better solution in future.

@brcrista
Copy link
Contributor

That makes sense -- just a matter of whether the time spent and added complexity is worth it. I'm going to close this as we're not planning on this work right now, but we'll keep this in mind if people keep asking for it.

@foolip
Copy link

foolip commented Jun 22, 2020

I was watching this thread without speaking up. I would also like a way to ensure that a specific version of Python is installed from another action, to simplify the requirements for others who depend on the action.

@asottile
Copy link

fwiw I've been pretty disappointed with the reusability of github actions, the design choices of having executable git repos make it really difficult to compose actions (instead they must be copy pasted everywhere) and with a lack of support for template repositories (such as with azure pipelines) means there really isn't any solution for things like this beyond adding more to copy paste

my particular usecase for having an in-process runnable setupPython is to patch a (imo) pretty glaring omission in this action: setting up in-development versions. the current implementation requires copy pasting two actions both with disparate complex conditionals. the ideal case would be a single action which just works (by calling out to setup-python in some way when it cannot handle the particular versions itself): deadsnakes/issues#123

@eregon
Copy link

eregon commented Jun 22, 2020

FWIW one way in another action we found to compose actions is to simply download the resulting dist/index.js file: ruby/setup-ruby#33 (comment)

@brcrista
Copy link
Contributor

Check out this issue for "composite actions:" actions/runner#438. There's also a working design proposal: actions/runner#554

I think this would address @peter-evans 's and @foolip 's scenarios. @asottile , this isn't the same templating model as Azure Pipelines, but I think that could address your use case also.

@bexnoss
Copy link

bexnoss commented Jan 31, 2022

This is currently broken. #306 changed the build to use ncc instead of tsc which bundles everything into one file without any exports.

@peter-evans' workaround is great but since the generated files are now tracked the build step isn't necessary anymore:

Add the repository as a dependency and specify a version.

npm i --save actions/setup-python#v2.3.1

It will get added to package.json like this.

  "dependencies": {
    "setup-python": "github:actions/setup-python#v2.3.1"
  }

It should be usable as in this example:

import * as core from '@actions/core'
import * as setupPython from 'setup-python/dist/setup'

async function run(): Promise<void> {
  try {
    const installed = await setupPython.findPythonVersion('3.x', 'x64')
    core.info(`Successfully setup ${installed.impl} (${installed.version})`)
  } catch (error) {
    core.setFailed(error.message)
  }
}

run()

@bexnoss bexnoss mentioned this issue Jan 31, 2022
2 tasks
@bexnoss
Copy link

bexnoss commented Jan 31, 2022

@peter-evans right, npm supports installing straight from a GitHub repo like you've found.

I think if we were publishing as an NPM package, we would want to do more work around giving the action a proper API and versioning the NPM package around that. So, if the existing path that npm provides is working well, I think that's a better alternative than taking on the dev and maintenance cost of a full-on package.

I think it would make sense to revisit this issue. The workaround broke and because there are no tests it wasn't noticed. If there was a NPM package this wouldn't have happened. I'd be happy to help, please let me know if there's anything I can do.

@bexnoss bexnoss mentioned this issue Feb 1, 2022
2 tasks
@brcrista brcrista reopened this Apr 12, 2022
@brcrista
Copy link
Contributor

brcrista commented Apr 12, 2022

I just put together an example using composite actions to set up a version of Python before running some other steps. @bexnoss @peter-evans does this help with what you're trying to do?

name: 'setup-python-composite'
description: 'Sample code for a composite action that requires some version of Python'
runs:
  using: "composite"
  steps:
  - uses: actions/setup-python@v2
    with:
      python-version: '3.x'

  - run: python --version
    shell: bash

  - uses: ./javascript-action

Here's an example workflow run with the action.

There's a few reasons I'd prefer to stick with composite actions rather than publishing an NPM package:

  • Composite actions is a first-class concept that should work with all actions. (Not all actions are implemented in JS.) If the first-class reusability concept isn't working we should improve it.
  • Avoid multiplying concepts unnecessarily
  • Avoid the extra maintenance cost of making sure the NPM package works edit: and keeping the versioning accurate

@peter-evans
Copy link
Author

@brcrista Looks like a good solution to me, and your reasoning makes sense. Composite actions appears to solve this quite well. 👍

As I mentioned here, I did end up rewriting my actions in Typescript, which avoided the problem altogether.

@bexnoss
Copy link

bexnoss commented Apr 19, 2022

@brcrista Composite actions do work and it does make sense to not publish an NPM package.

One downside of using composite actions is that afaik inputs have to be duplicated. Is there a way to automatically pass the inputs of the composite action to all sub actions?

@brcrista
Copy link
Contributor

@bexnoss you do need to pass inputs to the sub-actions manually using ${{ inputs.xxx }} (docs).

I'm going to close out this issue since it sounds like we're all set on the topic of the NPM package.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants