Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve package layout #37

Merged
merged 3 commits into from
Jul 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions opentelemetry-api/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
base_dir = os.path.dirname(__file__)

package_info = {}
with open(os.path.join(base_dir, "opentelemetry", "internal", "version.py")) as f:
with open(os.path.join(base_dir, "src", "opentelemetry", "internal", "version.py")) as f:
exec(f.read(), package_info)

setuptools.setup(
Expand All @@ -45,7 +45,8 @@
],
extras_require={},
license="Apache-2.0",
packages=setuptools.find_namespace_packages(include=["opentelemetry.*"]),
package_dir={"": "src"},
packages=setuptools.find_namespace_packages(where="src"),
url="https://github.com/open-telemetry/opentelemetry-python/tree/master/opentelemetry-api",
zip_safe=False,
)
4 changes: 4 additions & 0 deletions opentelemetry-sdk/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Changelog

## Unreleased
- Initial release
19 changes: 19 additions & 0 deletions opentelemetry-sdk/README.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
OpenTelemetry Python SDK
============================================================================

|pypi|

.. |pypi| image:: https://badge.fury.io/py/opentelemetry-sdk.svg
:target: https://pypi.org/project/opentelemetry-sdk/

Installation
------------

::

pip install opentelemetry-sdk

References
----------

* `OpenTelemetry Project <https://opentelemetry.io/>`_
51 changes: 51 additions & 0 deletions opentelemetry-sdk/setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
# Copyright 2019, OpenTelemetry Authors
#
# Licensed 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.

import os
import setuptools

base_dir = os.path.dirname(__file__)

package_info = {}
with open(os.path.join(base_dir, "src", "opentelemetry", "sdk", "version.py")) as f:
exec(f.read(), package_info)
Copy link
Member

Choose a reason for hiding this comment

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

Why use package_info instead of read/exec-ing right into globals?

Copy link
Member Author

@reyang reyang Jun 28, 2019

Choose a reason for hiding this comment

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

To prevent bad thing inside the version.py (later we might add more stuff such like SDK vendor info and rename the file to package_info.py) from polluting globals.


setuptools.setup(
name="opentelemetry-sdk",
version=package_info["__version__"], # noqa
author="OpenTelemetry Authors",
author_email="[email protected]",
classifiers=[
"Development Status :: 3 - Alpha",
"Intended Audience :: Developers",
"License :: OSI Approved :: Apache Software License",
"Programming Language :: Python",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.4",
"Programming Language :: Python :: 3.5",
"Programming Language :: Python :: 3.6",
"Programming Language :: Python :: 3.7",
],
description="OpenTelemetry Python SDK",
include_package_data=True,
long_description=open("README.rst").read(),
install_requires=[
Copy link
Member Author

Choose a reason for hiding this comment

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

opentelemetry-sdk will have dependency on opentelemetry-api, this will be a separate PR (we need to figure out how to install from local).

],
extras_require={},
license="Apache-2.0",
package_dir={"": "src"},
packages=setuptools.find_namespace_packages(where="src"),
url="https://github.com/open-telemetry/opentelemetry-python/tree/master/opentelemetry-sdk",
zip_safe=False,
)
15 changes: 15 additions & 0 deletions opentelemetry-sdk/src/opentelemetry/sdk/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright 2019, OpenTelemetry Authors
#
# Licensed 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.

from .version import __version__
Copy link
Member

Choose a reason for hiding this comment

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

Why do you need this?

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking about troubleshooting scenario, where we want to see which version of the SDK customers are using, we do need to distinguish SDK versions versus API versions.
For example, if the queue for traces are full, I'd expect we log something like:

2019-06-27T22:36:59Z OpenTelemetry API loaded (version=1.0.0)
2019-06-27T22:37:15Z OpenTelemetry SDK loaded (version=1.0.3)
2019-06-27T22:38:30Z traces queue full, started to drop spans
2019-06-27T22:38:50Z traces queue recovered

Copy link
Member

Choose a reason for hiding this comment

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

I mean why do you need to import it here instead of in whatever module uses it (the trace queue in your example)?

Copy link
Member Author

Choose a reason for hiding this comment

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

I see, my intent was to save some typing, so we can import sdk.__version__ instead of sdk.version.__version__. Later we can probably rename version.py to package_info.py if we have more stuff to put here.

15 changes: 15 additions & 0 deletions opentelemetry-sdk/src/opentelemetry/sdk/version.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Copyright 2019, OpenTelemetry Authors
#
# Licensed 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.

__version__ = "0.1.dev0"
Copy link
Member

Choose a reason for hiding this comment

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

Should you move the API version file out of internal/ to match?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean something like opentelemetry/api/version.py? We decided to put version.py in internal since there is no public interface. I'm open to suggestions.

Copy link
Member

Choose a reason for hiding this comment

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

I take #19 (review) to mean that we can't have a single top-level version file, but separate files under api/ and sdk/ should be fine.

4 changes: 2 additions & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ deps =
py37-mypy: mypy

commands =
py37-lint: pylint opentelemetry-api/opentelemetry/
py37-mypy: mypy opentelemetry-api/opentelemetry/
py37-lint: pylint opentelemetry-api/src/opentelemetry/
Copy link
Member Author

Choose a reason for hiding this comment

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

@c24t do you think it's good idea to run lint on opentelemetry-api (which includes setup.py, tests, examples)?

Copy link
Member

Choose a reason for hiding this comment

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

Sure, if it ends up being easier to blacklist dirs that shouldn't be linted than to whitelist every one that should.

py37-mypy: mypy opentelemetry-api/src/opentelemetry/