rename the http folder to a better name, than python #24
Conversation
|
@matzew: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: matzew The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow
bot
added
approved
Signed-off-by: Matthias Wessendorf <mwessend@redhat.com>
matzew
force-pushed
the
rename_folder_to_http
branch
from
86640ba to
3b6eb20
Compare
|
I don't think this will quite work as intended. I might be wrong, but this package is already published on PyPI as "func-python" (and imported as "func_python". By changing this, we might need to duplicate, or at least complicate, all our configuration or process to publish two separate packages; one for each of "http" and "cloudevents". Furthermore, it is my understanding the "Pythonic" idiom is to use files as modules, rather than directories (which is idiomatic in Go). Thus in Python one would expect an This alignment 1:1 from published package name to import name is the reason the current codebase does not require "packages" be explicitly defined in the pyproject.toml. I am fairly sure the current file hierarchy and package names are correct, and that the simplest option moving forward is to simply add "cloudevents.py" file right beside "http.py". This would be imported as I am happy to entertain proof otherwise, because I am not a Python packaging expert! |
| packages = [ | ||
| { include = "func_http", from = "src" }, | ||
| ] | ||
|
|
There was a problem hiding this comment.
The reason the folder is named func_python is such that it matches the package name as published on PyPI func-python. Hence the reason there's no need for an explicit packages section in the pyproject.toml
| @@ -4,7 +4,7 @@ | |||
| import signal | |||
| import threading | |||
| import time | |||
| from func_python.http import serve | |||
| from func_http.http import serve | |||
There was a problem hiding this comment.
With an http.py and a cloudevents.py file in the func_python directory, the two imports would be:
from func_python.http import serve for HTTP middleware and
from func_python.cloudevents import serve for CloudEVent middleware
That seems correct to me
|
/close thanks for the insights. Agree not needed |
|
@matzew: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| @@ -1,6 +1,6 @@ | |||
| import argparse | |||
| import logging | |||
| from func_python.http import serve | |||
| from func_http.http import serve | |||
There was a problem hiding this comment.
looking now at this I see that func_http.http is a bit unfortunate as well 😅
Changes
/kind
Fixes #
Release Note
Docs