Skip to content
This repository has been archived by the owner on Nov 30, 2023. It is now read-only.

Python updates #113

Merged
merged 6 commits into from
Aug 9, 2019
Merged

Python updates #113

merged 6 commits into from
Aug 9, 2019

Conversation

brettcannon
Copy link
Member

Bring the non-conda containers all in sync in regards to not running pip during container creation and updating their devcontainer.json for Pylint. For conda containers, just update devcontainer.json.

@brettcannon brettcannon requested a review from Chuxel August 8, 2019 22:31
Copy link
Member

@Chuxel Chuxel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we remove noop.txt in the python-2 and python-3-posgres folder?

Also, the root requirements.txt in the root of python-3-posgres can probably be deleted too, correct? The test project could do a preLaunchCommand to install requirements.txt if needed. The main reason that file was there was because it got built into the image.

Otherwise LGTM

@brettcannon
Copy link
Member Author

@Chuxel you're correct about the noop.txt; I thought I had got them but I obviously missed them.

As for the requirements.txt in python-3-postgres, you tell me. 😉 If you want the test project to actually function I would assume you would need the requirements.txt file. So I don't know what your expectations are for how much work it would take to make the test projects function.

@Chuxel
Copy link
Member

Chuxel commented Aug 9, 2019

As for the requirements.txt in python-3-postgres, you tell me. 😉 If you want the test project to actually function I would assume you would need the requirements.txt file. So I don't know what your expectations are for how much work it would take to make the test projects function.

But that doesn't actually get used now on build does it? What we've done for things like node is add a preLaunchTask in launch.json that runs yarn install. Here this could be:

.vscode/tasks.json

{
    "version": "2.0.0",
    "tasks": [
        {
            "label": "pip install",
            "type": "shell",
            "command": "pip install -r ./test-project/requirements.txt"
        }
    ]
}

.vscode/launch.json

{
    "version": "0.2.0",
    "configurations": [
        {
            "name": "Python: Django",
            "type": "python",
            "request": "launch",
            "program": "${workspaceFolder}/test-project/manage.py",
            "console": "integratedTerminal",
            "args": [
                "runserver",
                "0.0.0.0:5000",
                "--noreload",
                "--nothreading"
            ],
            "django": true,
            "preLaunchTask": "pip install"
        }
    ]
}

test-project/requirements.txt

Django
psycopg2

@brettcannon
Copy link
Member Author

@Chuxel Correct, pip install isn't run automatically anymore. So would you like me to add the examples you gave above to the PR?

@Chuxel
Copy link
Member

Chuxel commented Aug 9, 2019

@brettcannon Yeah I think that probably makes sense. We’ve had people ask why requirements.txt is in the root before.

@brettcannon
Copy link
Member Author

@Chuxel PTAL

@Chuxel
Copy link
Member

Chuxel commented Aug 9, 2019

@brettcannon LGTM!

@brettcannon brettcannon merged commit ae361c0 into microsoft:master Aug 9, 2019
@brettcannon brettcannon deleted the python-updates branch August 9, 2019 21:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants