Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Set up Python packaging for PyPI release. #197

Merged
merged 5 commits into from
Apr 12, 2019

Conversation

songy23
Copy link
Contributor

@songy23 songy23 commented Apr 1, 2019

@songy23
Copy link
Contributor Author

songy23 commented Apr 1, 2019

/cc @c24t @reyang @odeke-em I'm new to PyPI release setup, would appreciate it if you can help me review the release setup in this PR. Basically we want to release the gen-python files under https://github.com/census-instrumentation/opencensus-proto/tree/master/gen-python/opencensus to https://pypi.org/project/opencensus-proto/.

# See the License for the specific language governing permissions and
# limitations under the License.

__path__ = __import__('pkgutil').extend_path(__path__, __name__)
Copy link

Choose a reason for hiding this comment

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

This line is intended for namespace packages.
If there is no plan to make opencensus/proto/agent/trace/v1 a namespace (for example, having multiple packages sharing the opencensus.proto.agent.trace.v1 namespace), we can remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For v1 we won't have multiple packages. However we may have multiple packages (versions) share the upper level package. For example opencensus/proto/agent/trace/v1 and opencensus/proto/agent/trace/v2 will share opencensus/proto/agent/trace - kindly correct me if I'm wrong.

Removed all paths from */v1 for now.

Copy link

Choose a reason for hiding this comment

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

Yes, this (having v1 and v2) is what I was thinking, too :)

Copy link
Member

Choose a reason for hiding this comment

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

As it is now I don't think you need the incantation for namespace packages. If we release v1 and v2 as separate distributions, each with a dependency on a separate common distribution, then we'll need it.

What's our support story for v1 protos? If we're not updating them, could we release them as an earlier version of this library and drop support for later versions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we release v1 and v2 as separate distributions, each with a dependency on a separate common distribution, then we'll need it.

I think this is what we'll end up doing. In the future we'll have packages like opencensus/proto/common/v1 and opencensus/proto/common/v2. All the */v1 packages will depend on common/v1 and all v2 packages depend on common/v2.

What's our support story for v1 protos? If we're not updating them, could we release them as an earlier version of this library and drop support for later versions?

I don't have a clear answer to this but my idea is when v2 is out, we'll keep v1 protos there and continue releasing them with the same version number, though v1 protos won't be forward-compatible. (v2 protos are unlikely to be backwards-compatible either.)

Copy link
Member

Choose a reason for hiding this comment

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

That makes sense. You may just want to add the namespace package logic when you add a second setup.py then.

gen-python/setup.py Outdated Show resolved Hide resolved
Copy link

@reyang reyang left a comment

Choose a reason for hiding this comment

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

LGTM. This is super helpful!

@c24t
Copy link
Member

c24t commented Apr 1, 2019

FWIW we push opencensus-python to PyPI during CI. We may want to do the same thing here.

@songy23
Copy link
Contributor Author

songy23 commented Apr 12, 2019

Friendly ping @bogdandrutu

@songy23 songy23 merged commit 0994559 into census-instrumentation:master Apr 12, 2019
@songy23 songy23 deleted the setup-pypi branch April 12, 2019 22:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants