You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/07/11 11:37:29 UTC
[GitHub] [skywalking-python] jiang1997 opened a new pull request, #222: doc: add How to test locally
jiang1997 opened a new pull request, #222:
URL: https://github.com/apache/skywalking-python/pull/222
<!-- Uncomment the following checklist WHEN AND ONLY WHEN you're adding a new plugin -->
<!--
- [ ] Add a test case for the new plugin
- [ ] Add a CHANGELOG entry for the new plugin
- [ ] Add a component id in [the main repo](https://github.com/apache/skywalking/blob/master/oap-server/server-starter/src/main/resources/component-libraries.yml)
- [ ] Add a logo in [the UI repo](https://github.com/apache/skywalking-booster-ui/tree/main/src/assets/img/technologies)
- [ ] Rebuild the `requirements.txt` by running `tools/env/build_requirements_(linux|windows).sh`
- [ ] Rebuild the `Plugins.md` documentation by running `make doc-gen`
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #222: doc: add How to test locally
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #222:
URL: https://github.com/apache/skywalking-python/pull/222#discussion_r917844643
##########
docs/en/setup/faq/How-to-test-locally.md:
##########
@@ -0,0 +1,36 @@
+This guide assumes you just cloned the repo and are ready to make some changes.
+
+After cloning the repo, make sure you also have cloned the submodule for protocol. Otherwise, run the command below.
+```
+git submodule update --init
+```
+
+Then run ``make setup-test``. This will create virtual environments for python and generate the protocol folder needed for the agent.
+
+By now, you can do what you want.
+
+Let's get to the topic of how to test.
+The test process requires docker and docker-compose throughout. If you haven't installed them, please install them first.
+
+In the test process, the script will create several docker containers based on a particular docker image(apache/skywalking-python-agent:latest-plugin), but the current script does not create this docker image automatically. So we need to create manually.
+Here is the practice I employed.
+
+Create a file in the folder you just cloned.
+```
+FROM python:3.8
+
+WORKDIR /usr/src/app
+
+COPY . .
+
+RUN pip install -r requirements.txt
+RUN pip install -e .[test]
+then
+```
+
+Then run the command below to create the docker image we need later. Because the test process runs in the docker container, we need to apply the changes you did to the docker image. You may need to run this command whenever you change any files before the test.
+```
+docker build -t apache/skywalking-python-agent:latest-plugin . -f Dockerfile.test
+```
Review Comment:
All these can be simplified as
```
docker build --build-arg BASE_PYTHON_IMAGE=3.6 -t apache/skywalking-python-agent:latest-plugin --no-cache . -f tests/plugin/Dockerfile.plugin
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #222: doc: add How to test locally
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #222:
URL: https://github.com/apache/skywalking-python/pull/222#discussion_r917923272
##########
docs/en/setup/faq/How-to-test-locally.md:
##########
@@ -0,0 +1,36 @@
+This guide assumes you just cloned the repo and are ready to make some changes.
+
+After cloning the repo, make sure you also have cloned the submodule for protocol. Otherwise, run the command below.
+```
+git submodule update --init
+```
+
+Then run ``make setup-test``. This will create virtual environments for python and generate the protocol folder needed for the agent.
+
+By now, you can do what you want.
+
+Let's get to the topic of how to test.
+The test process requires docker and docker-compose throughout. If you haven't installed them, please install them first.
+
+In the test process, the script will create several docker containers based on a particular docker image(apache/skywalking-python-agent:latest-plugin), but the current script does not create this docker image automatically. So we need to create manually.
+Here is the practice I employed.
+
+Create a file in the folder you just cloned.
+```
+FROM python:3.8
+
+WORKDIR /usr/src/app
+
+COPY . .
+
+RUN pip install -r requirements.txt
+RUN pip install -e .[test]
+then
+```
+
+Then run the command below to create the docker image we need later. Because the test process runs in the docker container, we need to apply the changes you did to the docker image. You may need to run this command whenever you change any files before the test.
+```
+docker build -t apache/skywalking-python-agent:latest-plugin . -f Dockerfile.test
+```
Review Comment:
Don't forget to add license header for the new file
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on a diff in pull request #222: doc: add How to test locally
Posted by GitBox <gi...@apache.org>.
jiang1997 commented on code in PR #222:
URL: https://github.com/apache/skywalking-python/pull/222#discussion_r917904010
##########
docs/en/setup/faq/How-to-test-locally.md:
##########
@@ -0,0 +1,36 @@
+This guide assumes you just cloned the repo and are ready to make some changes.
+
+After cloning the repo, make sure you also have cloned the submodule for protocol. Otherwise, run the command below.
+```
+git submodule update --init
+```
+
+Then run ``make setup-test``. This will create virtual environments for python and generate the protocol folder needed for the agent.
+
+By now, you can do what you want.
+
+Let's get to the topic of how to test.
+The test process requires docker and docker-compose throughout. If you haven't installed them, please install them first.
+
+In the test process, the script will create several docker containers based on a particular docker image(apache/skywalking-python-agent:latest-plugin), but the current script does not create this docker image automatically. So we need to create manually.
+Here is the practice I employed.
+
+Create a file in the folder you just cloned.
+```
+FROM python:3.8
+
+WORKDIR /usr/src/app
+
+COPY . .
+
+RUN pip install -r requirements.txt
+RUN pip install -e .[test]
+then
+```
+
+Then run the command below to create the docker image we need later. Because the test process runs in the docker container, we need to apply the changes you did to the docker image. You may need to run this command whenever you change any files before the test.
+```
+docker build -t apache/skywalking-python-agent:latest-plugin . -f Dockerfile.test
+```
Review Comment:
Yea, when users have not run any make command, and there is no venv folder, this one-line command works as expected.
I observed when there is venv folder in ``ROOT``, this one-line command runs into error as below.
```
Step 6/7 : RUN cd /agent && make setup-test
---> Running in 8c58f3791fcb
venv/bin/python -m pip install grpcio --ignore-installed
/agent/venv/bin/python: No module named pip
make: *** [Makefile:33: setup] Error 1
The command '/bin/sh -c cd /agent && make setup-test' returned a non-zero code: 2
```
I guess because the existed venv is not configured for the environment inside docker image, but make script runs inside docker image do not aware of it.
After changing ``tests/plugin/Dockerfile.plugin`` as below, everything works fine.
```
ARG BASE_PYTHON_IMAGE
FROM python:${BASE_PYTHON_IMAGE}
ARG ROOT=.
WORKDIR /agent
ADD $ROOT /agent
RUN cd /agent && rm -rf venv && make setup-test
ENV PATH="/agent/venv/bin:$PATH"
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 merged pull request #222: doc: add How to test locally
Posted by GitBox <gi...@apache.org>.
kezhenxu94 merged PR #222:
URL: https://github.com/apache/skywalking-python/pull/222
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] jiang1997 commented on a diff in pull request #222: doc: add How to test locally
Posted by GitBox <gi...@apache.org>.
jiang1997 commented on code in PR #222:
URL: https://github.com/apache/skywalking-python/pull/222#discussion_r917968263
##########
docs/en/setup/faq/How-to-test-locally.md:
##########
@@ -0,0 +1,36 @@
+This guide assumes you just cloned the repo and are ready to make some changes.
+
+After cloning the repo, make sure you also have cloned the submodule for protocol. Otherwise, run the command below.
+```
+git submodule update --init
+```
+
+Then run ``make setup-test``. This will create virtual environments for python and generate the protocol folder needed for the agent.
+
+By now, you can do what you want.
+
+Let's get to the topic of how to test.
+The test process requires docker and docker-compose throughout. If you haven't installed them, please install them first.
+
+In the test process, the script will create several docker containers based on a particular docker image(apache/skywalking-python-agent:latest-plugin), but the current script does not create this docker image automatically. So we need to create manually.
+Here is the practice I employed.
+
+Create a file in the folder you just cloned.
+```
+FROM python:3.8
+
+WORKDIR /usr/src/app
+
+COPY . .
+
+RUN pip install -r requirements.txt
+RUN pip install -e .[test]
+then
+```
+
+Then run the command below to create the docker image we need later. Because the test process runs in the docker container, we need to apply the changes you did to the docker image. You may need to run this command whenever you change any files before the test.
+```
+docker build -t apache/skywalking-python-agent:latest-plugin . -f Dockerfile.test
+```
Review Comment:
Alright
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking-python] kezhenxu94 commented on a diff in pull request #222: doc: add How to test locally
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on code in PR #222:
URL: https://github.com/apache/skywalking-python/pull/222#discussion_r917922286
##########
docs/en/setup/faq/How-to-test-locally.md:
##########
@@ -0,0 +1,36 @@
+This guide assumes you just cloned the repo and are ready to make some changes.
+
+After cloning the repo, make sure you also have cloned the submodule for protocol. Otherwise, run the command below.
+```
+git submodule update --init
+```
+
+Then run ``make setup-test``. This will create virtual environments for python and generate the protocol folder needed for the agent.
+
+By now, you can do what you want.
+
+Let's get to the topic of how to test.
+The test process requires docker and docker-compose throughout. If you haven't installed them, please install them first.
+
+In the test process, the script will create several docker containers based on a particular docker image(apache/skywalking-python-agent:latest-plugin), but the current script does not create this docker image automatically. So we need to create manually.
+Here is the practice I employed.
+
+Create a file in the folder you just cloned.
+```
+FROM python:3.8
+
+WORKDIR /usr/src/app
+
+COPY . .
+
+RUN pip install -r requirements.txt
+RUN pip install -e .[test]
+then
+```
+
+Then run the command below to create the docker image we need later. Because the test process runs in the docker container, we need to apply the changes you did to the docker image. You may need to run this command whenever you change any files before the test.
+```
+docker build -t apache/skywalking-python-agent:latest-plugin . -f Dockerfile.test
+```
Review Comment:
Hi @jiang1997 thanks for pointing this out. I think you're right. We have another more canonical way to avoid this. Can you please add a file `.dockerignore` in root directory of this project and add the `venv` as its content? This should resolve the problem.
File `.dockerignore`:
```
venv
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org