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 2021/10/19 22:21:51 UTC
[GitHub] [skywalking-python] Superskyyy opened a new pull request #167: Cleanup and Python 3.10 support
Superskyyy opened a new pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167
Changelist:
Evaluates all plugin lib versions.
Adds a new utility to support Python version-specific tests (e.g. disable certain tests for certain Python versions).
Adds a plugin test table generator utility (in the tools folder, the output is to be copied into the plugins.md doc. for now.)
Enhances CI workflow to run more efficiently and new deadlink checker.
Supports Python 3.10.
Stricter flake8 lint rules.
Psycopg3(Postgres) support. - no need to add a component ID.
Chore
Note:
1. Sanic plugin is problematic, it has incompatible API changes in the next LTS + not supporting Python 310 yet.
2. Pymongo also has some problems with the next version.
3. Celery is not tested.
--
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] tom-pytel edited a comment on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
tom-pytel edited a comment on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947572463
The celery server does not support grpc because it forks server processes and grpc is not good with that so you will need to use the agent http protocol.
Server script:
```py
#!/usr/bin/env python3
# docker run --name=redis-devel --publish=6379:6379 --hostname=redis --restart=on-failure --detach redis:latest
# celery -A t_celery worker -l info
from celery import Celery
from time import sleep
from skywalking import agent
agent.start()
app = Celery('test', broker = 'redis://localhost:6379', backend = 'redis://localhost:6379')
@app.task
def reverse(text):
sleep(1)
return text[::-1]
```
Setup:
```
pip install redis celery "apache-skywalking[http]"
export SW_AGENT_PROTOCOL=http
export SW_AGENT_COLLECTOR_BACKEND_SERVICES="127.0.0.1:12800"
docker run --name=redis-devel --publish=6379:6379 --hostname=redis --restart=on-failure --detach redis:latest
celery -A t_celery worker -l info
```
In the same directory as the client (notice it imports the server module):
```py
#!/usr/bin/env python3
from skywalking import agent
agent.start()
import t_celery
print(t_celery.reverse.delay('ABC'))
print(t_celery.reverse('ABC'))
print(t_celery.reverse.apply_async(('ABC',)))
print(t_celery.reverse.apply(('ABC',)))
```
In the same directory but another terminal:
```
export SW_AGENT_PROTOCOL=http
export SW_AGENT_COLLECTOR_BACKEND_SERVICES="127.0.0.1:12800"
./t_celery_client.py
```
The final status of the jobs may not be instantaneous to update on the oap server, but all the exit celery requests should connect to to entry requests (except one spurious Local call which I just noticed but don't have time to track down where it came from right now).
NOTE: Ignore the redis stuff, or just disable the plugin, all you care about is the celery stuff.
--
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] Superskyyy commented on a change in pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on a change in pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#discussion_r732406371
##########
File path: tools/plugin_doc_gen.py
##########
@@ -0,0 +1,67 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""
+A tool to generate test matrix report for SkyWalking Python Plugins
+"""
+import pkgutil
+
+from skywalking.plugins import __path__ as plugins_path
+
+table_head = """
+Library | Python Version: Lib Version | Plugin Name
+| :--- | :--- | :--- |
+"""
+
+
+def generate_doc_table():
+ """
+ Generates a test matrix table to the current dir
+ # TODO inject into the plugins.md doc instead of manual copy
Review comment:
Ah, this isn't quite done yet, the intention is to make the test->doc pipeline fully automatic.
Manually evaluating the dependency once in a while is too painful(it took me hours to find broken lib versions) and it's very unreliable for users. So I'm gonna make your suggestion happen before 1.0.0 - https://github.com/apache/skywalking/issues/7701#issuecomment-921339584
--
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 change in pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#discussion_r732385051
##########
File path: .github/workflows/CI.yaml
##########
@@ -15,20 +15,44 @@
# limitations under the License.
#
-name: Build
+name: CI
on:
push:
branches:
- master
pull_request:
+# schedule: TODO: unpin minor lib versions and check weekly
+# - 0 0 0 ? * FRI *
+concurrency:
+ group: ci-it-${{ github.event.pull_request.number || github.ref }}
+ cancel-in-progress: true
jobs:
- Build:
+ license-and-lint:
+ name: License and Lint
runs-on: ubuntu-latest
+ steps:
+ - name: Checkout source codes
+ uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Check License
+ uses: apache/skywalking-eyes@49d536241d6fe8f92400702b08710514dc298cd4
+ env:
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ - name: Check license header
+ run: make license
Review comment:
Let's remove this in CI, `skywalking-eyes` already performs the same check
##########
File path: docs/en/contribution/PluginTest.md
##########
@@ -1,6 +1,10 @@
# Plugin Test
Plugin tests are required and should pass before a new plugin is able to merge into the master branch.
+Specify a support matrix in each plugin in the `skywalking/plugins` folder, along with their website links,
+the matrix and links will be used for plugin support table documentation generation for this doc [Plugins.md](../setup/Plugins.md).
+
+Use `tools/plugin_doc_gen.py` to generate a table and paste into `Plugins.md` after all test passes.
Review comment:
Let's add a check in CI to make sure this is not forgotten.
Example
https://github.com/apache/skywalking-cli/blob/b90255132f916f53eb90955cc8a6445b03a4bec3/Makefile#L127-L137
##########
File path: Makefile
##########
@@ -31,8 +31,12 @@ gen:
lint: clean
flake8 --version || python3 -m pip install flake8
- flake8 . --count --select=E9,F63,F7,F82 --show-source
- flake8 . --count --max-complexity=15 --max-line-length=120
+ flake8 . --count --show-source
+
+dev-check:
+ flake8 --version || python3 -m pip install flake8
+ flake8 . --count --show-source
+ python3 tools/check-license-header.py skywalking tests tools
Review comment:
Can be simplified?
```suggestion
dev-check: lint license
```
Would you please also migrate `tools/check-license-header.py` to `license-eye` (let's use Docker image from https://github.com/apache/skywalking-eyes/pkgs/container/skywalking-eyes%2Flicense-eye)?
##########
File path: tools/plugin_doc_gen.py
##########
@@ -0,0 +1,67 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""
+A tool to generate test matrix report for SkyWalking Python Plugins
+"""
+import pkgutil
+
+from skywalking.plugins import __path__ as plugins_path
+
+table_head = """
+Library | Python Version: Lib Version | Plugin Name
+| :--- | :--- | :--- |
+"""
+
+
+def generate_doc_table():
+ """
+ Generates a test matrix table to the current dir
+ # TODO inject into the plugins.md doc instead of manual copy
Review comment:
This is done IMO?
##########
File path: tools/plugin_doc_gen.py
##########
@@ -0,0 +1,67 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""
+A tool to generate test matrix report for SkyWalking Python Plugins
+"""
+import pkgutil
+
+from skywalking.plugins import __path__ as plugins_path
+
+table_head = """
+Library | Python Version: Lib Version | Plugin Name
+| :--- | :--- | :--- |
+"""
+
+
+def generate_doc_table():
+ """
+ Generates a test matrix table to the current dir
+ # TODO inject into the plugins.md doc instead of manual copy
+
+ Returns: None
+
+ """
+ table_entries = []
+ for importer, modname, ispkg in pkgutil.iter_modules(plugins_path):
+ plugin = importer.find_module(modname).load_module(modname)
+
+ plugin_support_matrix = plugin.support_matrix
+ plugin_support_links = plugin.link_vector
+ libs_tested = list(plugin_support_matrix.keys())
+ links_tested = plugin_support_links
+
+ for lib, link in zip(libs_tested, links_tested): # NOTE: maybe a two lib support like http.server + werkzeug
+ lib_entry = str(lib)
+ lib_link = link
+ version_vector = plugin_support_matrix[lib_entry] # type: dict
+ pretty_vector = ""
+ for python_version in version_vector: # e.g. {'>=3.10': ['2.5', '2.6'], '>=3.6': ['2.4.1', '2.5', '2.6']}
+ lib_versions = version_vector[python_version]
+ pretty_vector += f"Python {python_version} " \
+ f"- {str(lib_versions) if lib_versions else 'NOT SUPPORTED YET'}; "
Review comment:
`s/-/:/g`? In the doc (line 26) it's in form of `Python Version: Lib Version`, instead of `Python Version - Lib Version`
--
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] Superskyyy commented on a change in pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on a change in pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#discussion_r732398525
##########
File path: Makefile
##########
@@ -31,8 +31,12 @@ gen:
lint: clean
flake8 --version || python3 -m pip install flake8
- flake8 . --count --select=E9,F63,F7,F82 --show-source
- flake8 . --count --max-complexity=15 --max-line-length=120
+ flake8 . --count --show-source
+
+dev-check:
+ flake8 --version || python3 -m pip install flake8
+ flake8 . --count --show-source
+ python3 tools/check-license-header.py skywalking tests tools
Review comment:
Sure I can do the migration. For the dev-check command, it was for the convenience of development, else every time the clean step destroys my venv + skywalking-egg.. its a bit rough, but works for now.
--
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] tom-pytel edited a comment on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
tom-pytel edited a comment on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947572463
The celery server does not support grpc because it forks server processes and grpc is not good with that so you will need to use the agent http protocol.
Server script:
```py
#!/usr/bin/env python3
# docker run --name=redis-devel --publish=6379:6379 --hostname=redis --restart=on-failure --detach redis:latest
# celery -A t_celery worker -l info
from celery import Celery
from time import sleep
from skywalking import agent
agent.start()
app = Celery('test', broker = 'redis://localhost:6379', backend = 'redis://localhost:6379')
@app.task
def reverse(text):
sleep(1)
return text[::-1]
```
Setup:
```
pip install redis celery "apache-skywalking[http]"
export SW_AGENT_PROTOCOL=http
export SW_AGENT_COLLECTOR_BACKEND_SERVICES="127.0.0.1:12800"
docker run --name=redis-devel --publish=6379:6379 --hostname=redis --restart=on-failure --detach redis:latest
celery -A t_celery worker -l info
```
In the same directory as the server (notice it imports the server module):
```py
#!/usr/bin/env python3
from skywalking import agent
agent.start()
import t_celery
print(t_celery.reverse.delay('ABC'))
print(t_celery.reverse('ABC'))
print(t_celery.reverse.apply_async(('ABC',)))
print(t_celery.reverse.apply(('ABC',)))
```
In the same directory but another terminal:
```
export SW_AGENT_PROTOCOL=http
export SW_AGENT_COLLECTOR_BACKEND_SERVICES="127.0.0.1:12800"
./t_celery_client.py
```
The final status of the jobs may not be instantaneous to update on the oap server, but all the exit celery requests should connect to to entry requests (except one spurious Local call which I just noticed but don't have time to track down where it came from right now).
NOTE: Ignore the redis stuff, or just disable the plugin, all you care about is the celery stuff.
NOTE2: Correct fork handling only works with py 3.7+ since 3.6 does not have the hook, so not sure how http will work on that one (or if).
--
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] Superskyyy commented on a change in pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on a change in pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#discussion_r732402305
##########
File path: docs/en/contribution/PluginTest.md
##########
@@ -1,6 +1,10 @@
# Plugin Test
Plugin tests are required and should pass before a new plugin is able to merge into the master branch.
+Specify a support matrix in each plugin in the `skywalking/plugins` folder, along with their website links,
+the matrix and links will be used for plugin support table documentation generation for this doc [Plugins.md](../setup/Plugins.md).
+
+Use `tools/plugin_doc_gen.py` to generate a table and paste into `Plugins.md` after all test passes.
Review comment:
No problem, gonna do it in the next pr.
--
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] tom-pytel commented on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
tom-pytel commented on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947572463
The celery server does not support grpc because it forks server processes and grpc is not good with that so you will need to use the agent http protocol.
Server script:
```py
#!/usr/bin/env python3
# docker run --name=redis-devel --publish=6379:6379 --hostname=redis --restart=on-failure --detach redis:latest
# celery -A t_celery worker -l info
from celery import Celery
from time import sleep
from skywalking import agent
agent.start()
app = Celery('test', broker = 'redis://localhost:6379', backend = 'redis://localhost:6379')
@app.task
def reverse(text):
sleep(1)
return text[::-1]
```
Setup:
```
pip install redis celery "apache-skywalking[http]"
export SW_AGENT_PROTOCOL=http
export SW_AGENT_COLLECTOR_BACKEND_SERVICES="127.0.0.1:12800"
docker run --name=redis-devel --publish=6379:6379 --hostname=redis --restart=on-failure --detach redis:latest
celery -A t_celery worker -l info
```
In the same directory as the client (notice it imports the server module):
```py
#!/usr/bin/env python3
from skywalking import agent
agent.start()
import t_celery
print(t_celery.reverse.delay('ABC'))
print(t_celery.reverse('ABC'))
print(t_celery.reverse.apply_async(('ABC',)))
print(t_celery.reverse.apply(('ABC',)))
```
In the same directory but another terminal:
```
export SW_AGENT_PROTOCOL=http
export SW_AGENT_COLLECTOR_BACKEND_SERVICES="127.0.0.1:12800"
./t_celery_client.py
```
The final status of the jobs may not be instantaneous to update on the oap server, but all the exit celery requests should connect to to entry requests (except one spurious Local call which I just noticed but don't have time to track down where it came from right now).
--
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] tom-pytel commented on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
tom-pytel commented on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947207448
> **Changelist:**
>
> 7. Psycopg3.0 (psycopg) support. - no need to add a component ID.
psycopg 3.0 has added an async connection and cursor object which are not instrumented in a simple copy of the psycopg2 plugin. Note should be made of this. I am on a different project now but if I find the time I will see about adding this.
> **Note:**
>
> 3. Celery is not tested.
My fault, I was short on time. If you want a simple celery script to turn into a full test then I can provide that.
--
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] Superskyyy commented on a change in pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on a change in pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#discussion_r732433419
##########
File path: Makefile
##########
@@ -31,8 +31,12 @@ gen:
lint: clean
flake8 --version || python3 -m pip install flake8
- flake8 . --count --select=E9,F63,F7,F82 --show-source
- flake8 . --count --max-complexity=15 --max-line-length=120
+ flake8 . --count --show-source
+
+dev-check:
+ flake8 --version || python3 -m pip install flake8
+ flake8 . --count --show-source
+ python3 tools/check-license-header.py skywalking tests tools
Review comment:
Also doing the migration in next PR.
--
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] wu-sheng commented on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947199320
> Sanic plugin is problematic, it has incompatible API changes in the next LTS + not supporting Python 310 yet.
Does python agent have version identify capability? Or we have to choose?
--
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] Superskyyy commented on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947669003
> The celery server does not support grpc because it forks server processes and grpc is not good with that so you will need to use the agent http protocol.
>
> Server script:
>
> ```python
> #!/usr/bin/env python3
>
> # docker run --name=redis-devel --publish=6379:6379 --hostname=redis --restart=on-failure --detach redis:latest
> # celery -A t_celery worker -l info
>
> from celery import Celery
> from time import sleep
>
> from skywalking import agent
> agent.start()
>
> app = Celery('test', broker = 'redis://localhost:6379', backend = 'redis://localhost:6379')
>
> @app.task
> def reverse(text):
> sleep(1)
> return text[::-1]
> ```
>
> Setup:
>
> ```
> pip install redis celery "apache-skywalking[http]"
> export SW_AGENT_PROTOCOL=http
> export SW_AGENT_COLLECTOR_BACKEND_SERVICES="127.0.0.1:12800"
> docker run --name=redis-devel --publish=6379:6379 --hostname=redis --restart=on-failure --detach redis:latest
> celery -A t_celery worker -l info
> ```
>
> In the same directory as the server (notice it imports the server module):
>
> ```python
> #!/usr/bin/env python3
>
> from skywalking import agent
> agent.start()
>
> import t_celery
>
> print(t_celery.reverse.delay('ABC'))
> print(t_celery.reverse('ABC'))
> print(t_celery.reverse.apply_async(('ABC',)))
> print(t_celery.reverse.apply(('ABC',)))
> ```
>
> In the same directory but another terminal:
>
> ```
> export SW_AGENT_PROTOCOL=http
> export SW_AGENT_COLLECTOR_BACKEND_SERVICES="127.0.0.1:12800"
> ./t_celery_client.py
> ```
>
> The final status of the jobs may not be instantaneous to update on the oap server, but all the exit celery requests should connect to to entry requests (except one spurious Local call which I just noticed but don't have time to track down where it came from right now).
>
> NOTE: Ignore the redis stuff, or just disable the plugin, all you care about is the celery stuff.
>
> NOTE2: Correct fork handling only works with py 3.7+ since 3.6 does not have the hook, so not sure how http will work on that one (or if).
Thank you! I will look into this.
--
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] Superskyyy edited a comment on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy edited a comment on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947210661
> > Sanic plugin is problematic, it has incompatible API changes in the next LTS + not supporting Python 310 yet.
>
> Does python agent have version identify capability? Or we have to choose?
I implemented a version matrix to tackle most parts of the problem, it needs to be improved. In my mind, it should be able to pull from the Pypi registry to dynamically test the latest N releases + generate doc. That way it is more practical for maintenance and better coverage without much pain.
But for method signature change it will need another version check during runtime and pick appropriate instrumentation, didn't implement this because the old version is going away soon anyway.
--
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] Superskyyy commented on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947210661
> > Sanic plugin is problematic, it has incompatible API changes in the next LTS + not supporting Python 310 yet.
>
> Does python agent have version identify capability? Or we have to choose?
I implemented a version matrix to tackle most parts of the problem, it needs to be improved. In my mind, it should be able to pull from the Pypi registry to dynamically test the latest n releases + generate doc.
But for method signature change it will need another version check during runtime and pick appropriate instrumentation, didn't implement this because the old version is going away soon anyway.
--
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] Superskyyy commented on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947211742
> > **Changelist:**
> >
> > 1. Psycopg3.0 (psycopg) support. - no need to add a component ID.
>
> psycopg 3.0 has added an async connection and cursor object which are not instrumented in a simple copy of the psycopg2 plugin. Note should be made of this. I am on a different project now but if I find the time I will see about adding this.
>
> > **Note:**
> >
> > 1. Celery is not tested.
>
> My fault, I was short on time. If you want a simple celery script to turn into a full test then I can provide that.
I see, appreciated. I'm not familiar with celery, it will be nice if you could provide a simple script. I will make it a full test.
--
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] Superskyyy commented on a change in pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on a change in pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#discussion_r732425822
##########
File path: tools/plugin_doc_gen.py
##########
@@ -0,0 +1,67 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""
+A tool to generate test matrix report for SkyWalking Python Plugins
+"""
+import pkgutil
+
+from skywalking.plugins import __path__ as plugins_path
+
+table_head = """
+Library | Python Version: Lib Version | Plugin Name
+| :--- | :--- | :--- |
+"""
+
+
+def generate_doc_table():
+ """
+ Generates a test matrix table to the current dir
+ # TODO inject into the plugins.md doc instead of manual copy
+
+ Returns: None
+
+ """
+ table_entries = []
+ for importer, modname, ispkg in pkgutil.iter_modules(plugins_path):
+ plugin = importer.find_module(modname).load_module(modname)
+
+ plugin_support_matrix = plugin.support_matrix
+ plugin_support_links = plugin.link_vector
+ libs_tested = list(plugin_support_matrix.keys())
+ links_tested = plugin_support_links
+
+ for lib, link in zip(libs_tested, links_tested): # NOTE: maybe a two lib support like http.server + werkzeug
+ lib_entry = str(lib)
+ lib_link = link
+ version_vector = plugin_support_matrix[lib_entry] # type: dict
+ pretty_vector = ""
+ for python_version in version_vector: # e.g. {'>=3.10': ['2.5', '2.6'], '>=3.6': ['2.4.1', '2.5', '2.6']}
+ lib_versions = version_vector[python_version]
+ pretty_vector += f"Python {python_version} " \
+ f"- {str(lib_versions) if lib_versions else 'NOT SUPPORTED YET'}; "
Review comment:
Fixed, also made it to a whole doc generator instead of table generator.
--
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 #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
kezhenxu94 merged pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167
--
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] Superskyyy edited a comment on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy edited a comment on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947210661
> > Sanic plugin is problematic, it has incompatible API changes in the next LTS + not supporting Python 310 yet.
>
> Does python agent have version identify capability? Or we have to choose?
I implemented a version matrix to tackle most parts of the problem, it needs to be improved. In my mind, it should be able to pull from the Pypi registry to dynamically test the latest N releases + generate doc. That way it is more practical for maintenance and better coverage without much pain.
But for method signature change it will need another version check during runtime and pick appropriate instrumentation, didn't implement this because the old version is going away soon anyway. But I suppose we are gonna need it in the near future for robustness.
--
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] Superskyyy commented on a change in pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on a change in pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#discussion_r732398525
##########
File path: Makefile
##########
@@ -31,8 +31,12 @@ gen:
lint: clean
flake8 --version || python3 -m pip install flake8
- flake8 . --count --select=E9,F63,F7,F82 --show-source
- flake8 . --count --max-complexity=15 --max-line-length=120
+ flake8 . --count --show-source
+
+dev-check:
+ flake8 --version || python3 -m pip install flake8
+ flake8 . --count --show-source
+ python3 tools/check-license-header.py skywalking tests tools
Review comment:
Sure I can do the migration. For the dev-check command, it was for the convenience of development, else every time the clean step destroys my venv + skywalking-egg..
##########
File path: .github/workflows/CI.yaml
##########
@@ -15,20 +15,44 @@
# limitations under the License.
#
-name: Build
+name: CI
on:
push:
branches:
- master
pull_request:
+# schedule: TODO: unpin minor lib versions and check weekly
+# - 0 0 0 ? * FRI *
+concurrency:
+ group: ci-it-${{ github.event.pull_request.number || github.ref }}
+ cancel-in-progress: true
jobs:
- Build:
+ license-and-lint:
+ name: License and Lint
runs-on: ubuntu-latest
+ steps:
+ - name: Checkout source codes
+ uses: actions/checkout@v2
+ with:
+ submodules: true
+ - name: Check License
+ uses: apache/skywalking-eyes@49d536241d6fe8f92400702b08710514dc298cd4
+ env:
+ GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ - name: Check license header
+ run: make license
Review comment:
Yeah, I was thinking why we need it, removing.
--
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] tom-pytel edited a comment on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
tom-pytel edited a comment on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947572463
The celery server does not support grpc because it forks server processes and grpc is not good with that so you will need to use the agent http protocol.
Server script:
```py
#!/usr/bin/env python3
# docker run --name=redis-devel --publish=6379:6379 --hostname=redis --restart=on-failure --detach redis:latest
# celery -A t_celery worker -l info
from celery import Celery
from time import sleep
from skywalking import agent
agent.start()
app = Celery('test', broker = 'redis://localhost:6379', backend = 'redis://localhost:6379')
@app.task
def reverse(text):
sleep(1)
return text[::-1]
```
Setup:
```
pip install redis celery "apache-skywalking[http]"
export SW_AGENT_PROTOCOL=http
export SW_AGENT_COLLECTOR_BACKEND_SERVICES="127.0.0.1:12800"
docker run --name=redis-devel --publish=6379:6379 --hostname=redis --restart=on-failure --detach redis:latest
celery -A t_celery worker -l info
```
In the same directory as the client (notice it imports the server module):
```py
#!/usr/bin/env python3
from skywalking import agent
agent.start()
import t_celery
print(t_celery.reverse.delay('ABC'))
print(t_celery.reverse('ABC'))
print(t_celery.reverse.apply_async(('ABC',)))
print(t_celery.reverse.apply(('ABC',)))
```
In the same directory but another terminal:
```
export SW_AGENT_PROTOCOL=http
export SW_AGENT_COLLECTOR_BACKEND_SERVICES="127.0.0.1:12800"
./t_celery_client.py
```
The final status of the jobs may not be instantaneous to update on the oap server, but all the exit celery requests should connect to to entry requests (except one spurious Local call which I just noticed but don't have time to track down where it came from right now).
NOTE: Ignore the redis stuff, or just disable the plugin, all you care about is the celery stuff.
NOTE2: Correct fork handling only works with py 3.7+ since 3.6 does not have the hook, so not sure how http will work on that one (or if).
--
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] Superskyyy commented on a change in pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on a change in pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#discussion_r732428070
##########
File path: tools/plugin_doc_gen.py
##########
@@ -0,0 +1,67 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""
+A tool to generate test matrix report for SkyWalking Python Plugins
+"""
+import pkgutil
+
+from skywalking.plugins import __path__ as plugins_path
+
+table_head = """
+Library | Python Version: Lib Version | Plugin Name
+| :--- | :--- | :--- |
+"""
+
+
+def generate_doc_table():
+ """
+ Generates a test matrix table to the current dir
+ # TODO inject into the plugins.md doc instead of manual copy
Review comment:
removed the todo
--
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] Superskyyy commented on pull request #167: Cleanup and Python 3.10 support
Posted by GitBox <gi...@apache.org>.
Superskyyy commented on pull request #167:
URL: https://github.com/apache/skywalking-python/pull/167#issuecomment-947248993
The noted problems will be addressed in the following PRs, this one is becoming way to big.
--
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