You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by "Superskyyy (via GitHub)" <gi...@apache.org> on 2023/01/30 06:11:25 UTC
[GitHub] [skywalking-python] Superskyyy opened a new pull request, #273: Remove configuration debts before we release 1.0.0
Superskyyy opened a new pull request, #273:
URL: https://github.com/apache/skywalking-python/pull/273
A considerable number of options are not well-formated in terms of their environment variable name. which brings headache to both maintainance and users.
**This PR introduces breaking changes since configurations are renamed, this is explicitely stated at top of v1.0.0 changelog.**
Now I also added a documentation generator and checker to enforce a good and fully automatic way to produce configuration docuementation on the website.
Changes in this PR:
1. Renamed a bunch of configs mainly envvar names to follow `SW_AGENT_`, also prioritized important configs.
2. Added doc generator, config-envvar sync checker, ci config checker
3. Fix a potential undetected problem in profiler code
- [ ] If this pull request closes/resolves/fixes an existing issue, replace the issue url. Closes: <URL to main repo issue>
- [x] Update the [`CHANGELOG.md`](https://github.com/apache/skywalking-python/blob/master/CHANGELOG.md).
--
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 diff in pull request #273: Remove configuration debts before we release 1.0.0
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on code in PR #273:
URL: https://github.com/apache/skywalking-python/pull/273#discussion_r1096594060
##########
tests/plugin/docker-compose.base.yml:
##########
@@ -35,10 +35,9 @@ services:
agent:
image: apache/skywalking-python-agent:latest-plugin
environment:
- SW_AGENT_COLLECTOR_BACKEND_SERVICES: collector:19876
Review Comment:
oO I didn't see this thread until now, do you think we should tweak some naming to sync with java agent?
--
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 #273: Remove configuration debts before we release 1.0.0
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on PR #273:
URL: https://github.com/apache/skywalking-python/pull/273#issuecomment-1408051735
Fixing a deadlink.
--
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 #273: Remove configuration debts before we release 1.0.0
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #273:
URL: https://github.com/apache/skywalking-python/pull/273#discussion_r1090264445
##########
tests/plugin/docker-compose.base.yml:
##########
@@ -35,10 +35,9 @@ services:
agent:
image: apache/skywalking-python-agent:latest-plugin
environment:
- SW_AGENT_COLLECTOR_BACKEND_SERVICES: collector:19876
Review Comment:
Some of the env var names (like this one) were deliberately given that names to keep it consistent with other agent's configuration, although I agree you rename to a better one.
--
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 diff in pull request #273: Remove configuration debts before we release 1.0.0
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on code in PR #273:
URL: https://github.com/apache/skywalking-python/pull/273#discussion_r1096613030
##########
tests/plugin/docker-compose.base.yml:
##########
@@ -35,10 +35,9 @@ services:
agent:
image: apache/skywalking-python-agent:latest-plugin
environment:
- SW_AGENT_COLLECTOR_BACKEND_SERVICES: collector:19876
Review Comment:
>
Yeah it doens't have to be the same just I think it's a convention to have that consistent env_prefix_var as I seen in other python libraries. I will make some changes tmr to make important envvars back in sync with Java agent, meanwhile I think it's okay to sacrafice the length consideration in exchange for consistency (to be the same as envvar).
--
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 merged pull request #273: Remove configuration debts before we release 1.0.0
Posted by "wu-sheng (via GitHub)" <gi...@apache.org>.
wu-sheng merged PR #273:
URL: https://github.com/apache/skywalking-python/pull/273
--
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 diff in pull request #273: Remove configuration debts before we release 1.0.0
Posted by "Superskyyy (via GitHub)" <gi...@apache.org>.
Superskyyy commented on code in PR #273:
URL: https://github.com/apache/skywalking-python/pull/273#discussion_r1096594583
##########
tests/plugin/docker-compose.base.yml:
##########
@@ -35,10 +35,9 @@ services:
agent:
image: apache/skywalking-python-agent:latest-plugin
environment:
- SW_AGENT_COLLECTOR_BACKEND_SERVICES: collector:19876
Review Comment:
> oO I didn't see this thread until now, do you think we should tweak some naming to sync with java agent?
From the point of python keyed parameter feature, it maybe inconvinient to have a long name, so I generally shortened them a bit. Anyway I will think about it and update some if needed. Next version I suppose we will provide an actual config.toml file instead of fully relying on env vars or code changes.
--
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 #273: Remove configuration debts before we release 1.0.0
Posted by "kezhenxu94 (via GitHub)" <gi...@apache.org>.
kezhenxu94 commented on code in PR #273:
URL: https://github.com/apache/skywalking-python/pull/273#discussion_r1096608660
##########
tests/plugin/docker-compose.base.yml:
##########
@@ -35,10 +35,9 @@ services:
agent:
image: apache/skywalking-python-agent:latest-plugin
environment:
- SW_AGENT_COLLECTOR_BACKEND_SERVICES: collector:19876
Review Comment:
> > oO I didn't see this thread until now, do you think we should tweak some naming to sync with java agent?
>
>
>
> From the point of python keyed parameter feature, it maybe inconvinient to have a long name, so I generally shortened them a bit.
Can we shorten the parameter name in code while keep the original env var name? I think the parameter name doesn't have to be the same as env var name right?
> Anyway I will think about it and update some if needed. Next version I suppose we will provide an actual config.toml file instead of fully relying on env vars or code changes.
We can have a toml file for configuration for sure. But all the configurations in that file should be overridable via env var, that would be convenient in Docker/Kubernetes use case.
--
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