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

Are the Python steps in azure-webapps-python.yml pointless? #1669

Open
jakeprice-me opened this issue Aug 9, 2022 · 15 comments
Open

Are the Python steps in azure-webapps-python.yml pointless? #1669

jakeprice-me opened this issue Aug 9, 2022 · 15 comments
Labels
question Further information is requested

Comments

@jakeprice-me
Copy link

Hello, I wonder if somebody would be able to clarify why you include the below steps in deployments/azure-webapps-python.yml

We're experimenting with Azure App Service, and this is the GitHub Actions workflow that gets automatically created for us when we link our repository to the deployment center.

      - name: Set up Python version
        uses: actions/setup-python@v3.0.0
        with:
          python-version: ${{ env.PYTHON_VERSION }}
          cache: 'pip'

      - name: Create and start virtual environment
        run: |
          python -m venv venv
          source venv/bin/activate
      - name: Install dependencies
        run: pip install -r requirements.txt
        
      # Optional: Add step to run tests here (PyTest, Django test suites, etc.)
      
      - name: Upload artifact for deployment jobs
        uses: actions/upload-artifact@v3
        with:
          name: python-app
          path: |
            . 
            !venv/

The reason I ask is because unless I am missing something obvious, are these steps not pointless?

As far as I am aware, the steps are running on a GitHub Actions runner (running Ubuntu), not on the App Service container (running Debian) that will run the web application, and because you can't easily copy a venv directory to another machine, why bother creating and activating a virtual environment, and then installing pip packages?

In fact in the very last step above, you explicitly exclude the created venv directory from being included in the final artifact that gets deployed to App Service? 🤔 Is it running as some kind of test, to make sure the packages install before deploying? If that's why you're doing it, I still wonder where the value in that is? The GitHub Actions runner is running Ubuntu, and the App Service container is running Debian 10/11 - so you couldn't guarantee you wouldn't run into issues when you deploy, even if the workflow passes?

I've seen this in a few places, similar steps are used in the sample workflow here and another sample from an App Service team blog post by @JasonFreeberg here.

Any clarity on why you're doing this would be super helpful, because I don't think the docs make it clear at all.

@JasonFreeberg
Copy link
Contributor

The installation steps are there to precede the test step. Sad thing about Python is that AFAIK there isn't a standard testing framework or command (such as Node's npm run test or Maven's standard commands) so we couldn't include the testing command in the workflow like we do for Node.js/Java/.NET.

Yes the venv is excluded because venvs aren't portable across machines like you noted.

I agree that it would be better if the workflow ran on Debian, but GitHub Actions does not have a built-in runner for Debian at this time.

I can file an issue on myself to update the docs to explain this in more detail. One curious question: when you say "docs", were you following the docs on the GitHub site or Microsoft Docs site?

@jakeprice-me
Copy link
Author

Thanks for coming back so quickly @JasonFreeberg, that makes sense.

It was a combination of the Microsoft Docs site, and the blog post you wrote - perhaps some clarity around what the purpose of those steps are might be helpful?

Specific docs that come to mind are (will update if I remember any others):

Maybe the sample could also be a bit clearer on what those steps are for?

@webknjaz
Copy link

In fact in the very last step above, you explicitly exclude the created venv directory from being included in the final artifact that gets deployed to App Service?

Moreover, that venv is not actually used. The activate script exports a few environment variables, including the modified $PATH but is it is now, it's scoped to that step and there's nothing happening after sourcing it.

The next step is executed in the scope of the global interpreter environment — nothing is installed into that venv/ folder. If one wanted to make use of that venv, they'd have to invoke something like venv/bin/pip or venv/bin/python -m pip instead of just pip that is being looked up on the $PATH that actions/setup-python has previously set up.

By the way, the venv could've been created outside of the working directory and there wouldn't be a need to exclude it.

Sad thing about Python is that AFAIK there isn't a standard testing framework or command

This is not entirely true. There is a de-facto standard which is using pytest. Even though, some apps use things like ./manage.py test, invoking pytest would be pretty much universal. OTOH, many projects wrap test invocations with tox having to run tests against matrixes of envs and wanting to preserve the same interface for running things locally.

@github-actions
Copy link

This issue has become stale and will be closed automatically within a period of time. Sorry about that.

@webknjaz
Copy link

It still looks broken

@Danja30

This comment was marked as abuse.

@github-actions
Copy link

This issue has become stale and will be closed automatically within a period of time. Sorry about that.

@webknjaz
Copy link

Unstale

@JamesMGreene
Copy link
Contributor

@JasonFreeberg Any reply to the latest comments? 👂🏻

I'm not personally too familiar with running Python setups, nor doing so on Azure, so definitely interested to hear your opinion here as these workflows appear to have been sourced from you in the first place (indirectly via PR #1194). 🙇🏻

@JamesMGreene JamesMGreene added the question Further information is requested label Jul 13, 2023
@github-actions
Copy link

This issue has become stale and will be closed automatically within a period of time. Sorry about that.

@webknjaz
Copy link

Unstale

Copy link

This issue has become stale and will be closed automatically within a period of time. Sorry about that.

@webknjaz
Copy link

unstale

Copy link

This issue has become stale and will be closed automatically within a period of time. Sorry about that.

@webknjaz
Copy link

unstale

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

No branches or pull requests

7 participants
@JamesMGreene @webknjaz @JasonFreeberg @Phantsure @jakeprice-me @Danja30 and others