apache / pulsar-client-python

Apache Pulsar Python client library
https://pulsar.apache.org/
Apache License 2.0
50 stars 42 forks source link

Add a version attribute #81

Closed erichare closed 1 year ago

erichare commented 1 year ago

This PR addresses #27 - Specifically, it does the following:

In the end, this should hopefully maintain existing uses of the version file without breakage, but also provide for the requested version attribute that can be accessed in the standard way. (We could also look to using a dependency package like importlib, see the recommendations: https://peps.python.org/pep-0396/)

eolivelli commented 1 year ago

Can you lease describe the modifications?

erichare commented 1 year ago

Can you lease describe the modifications?

@eolivelli thanks, i just updated the description in the parent comment to describe what is being done!

BewareMyPower commented 1 year ago

This PR looks like to use the 4th method of https://packaging.python.org/en/latest/guides/single-sourcing-package-version/#single-sourcing-the-version, but as the warning says:

With this approach you must make sure that the VERSION file is included in all your source and binary distributions

The version.txt should be packaged into the Python wheel. I'm not familiar with the PyPI setup.py and you might need to search for how to package it.

If I were you, I would adopt the 3rd method, which is simple and clear.

Create an __about__.py, which defines a __version__ global variable, in the pulsar/ directory:

__version__='3.1.0a1'

Then, in pulsar/__init__.py you can get the version by:

from pulsar.__about__ import __version__

And in setup.py you can change the get_version function to:

def get_version():
    version = {}
    with open("./pulsar/__about__.py") as fp:
        exec(fp.read(), version)
    return version['__version__']

References:

erichare commented 1 year ago

This PR looks like to use the 4th method of https://packaging.python.org/en/latest/guides/single-sourcing-package-version/#single-sourcing-the-version, but as the warning says:

With this approach you must make sure that the VERSION file is included in all your source and binary distributions

The version.txt should be packaged into the Python wheel. I'm not familiar with the PyPI setup.py and you might need to search for how to package it.

If I were you, I would adopt the 3rd method, which is simple and clear.

Create an __about__.py, which defines a __version__ global variable, in the pulsar/ directory:

__version__='3.1.0a1'

Then, in pulsar/__init__.py you can get the version by:

from pulsar.__about__ import __version__

And in setup.py you can change the get_version function to:

def get_version():
    version = {}
    with open("./pulsar/__about__.py") as fp:
        exec(fp.read(), version)
    return version['__version__']

References:

@BewareMyPower i totally agree with this and appreciate your feedback. For reference, my motivation in choosing the fourth method was because there is already a version.txt file that was included in the repo - which was being read by the pkg/mac/build-mac-wheels.sh script. I was trying to keep that component as similar as possible. With that said, I think this is overall better and more pythonic, and i've updated the bash script to continue to read in the version info (assuming the structure of the variable and its definition provided).

I've pushed the latest commit, if you have a moment to take a look and make sure this looks good, it would be much appreciated. Thank you!