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

Generate instrumentation packages setup.py files #474

Merged
merged 3 commits into from
Apr 30, 2021

Conversation

owais
Copy link
Contributor

@owais owais commented Apr 27, 2021

Description

All instrumentations packages have almost exactly same setup.py files. This commit adds a python script that generates it from a source template. This dramatically reduces the time and effort required to update all instrumentation setup.py files, and also reduces chances of making manual mistakes.

I'm working on another feature where I needed to update setup.py files for all insrumentations. I'll use this once it is merged.

Another alternative is to take out common code as yet another package and add it as a setup_requires dependency so it is installed in target envs before setup.py runs but this adds another runtime dependency for users and doesn't give them any benefits. It's also not heavily used in the eco-system so non standard tools (other than pip) may not work well with it.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Tooling enhancement

Does This PR Require a Core Repo Change?

  • Yes. - Link to PR:
  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@owais owais force-pushed the generate-setuppy branch from 0629f27 to 30f9d92 Compare April 27, 2021 13:53
@owais owais added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Apr 27, 2021
@owais owais force-pushed the generate-setuppy branch 10 times, most recently from d47e628 to 9a20509 Compare April 27, 2021 18:36
@owais owais marked this pull request as ready for review April 27, 2021 18:37
@owais owais requested review from a team, aabmass and ocelotl and removed request for a team April 27, 2021 18:37
@owais owais force-pushed the generate-setuppy branch 3 times, most recently from ebd5239 to ab36898 Compare April 27, 2021 18:56
@owais owais mentioned this pull request Apr 28, 2021
11 tasks
@owais owais requested a review from srikanthccv April 28, 2021 09:10
@@ -11,6 +11,12 @@
# 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.


# DO NOT EDIT. THIS FILE WAS AUTOGENERATED FROM templaes/wsgi.
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like a typo sneaked into the generated code, though it doesnt appear in the generator 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, looks like I updated the generator but didn't run it before checking in.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Would love to see this extended to include the version file at some point as well, though I don't know if the instrumentation versions will always been the same or not.

@@ -11,6 +11,12 @@
# 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.


# DO NOT EDIT. THIS FILE WAS AUTOGENERATED FROM templaes/urllib3.
Copy link
Contributor

Choose a reason for hiding this comment

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

should this say it was generated from instrumentation_setup.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. This is incorrect. Fixing.



# DO NOT EDIT. THIS FILE WAS AUTOGENERATED FROM templaes/wsgi.
# Run `python scripts/generate_setuppy.py` to regenerate.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I see why you named this the way you did, but I would rename the script generate_setup.py, it's easier to read.

@owais owais force-pushed the generate-setuppy branch 3 times, most recently from 90a11ec to 538c3f1 Compare April 28, 2021 18:07
@owais owais requested a review from codeboten April 28, 2021 18:07
All instrumentations packages have almost exactly same setup.py files.
This commit adds a python script that generates it from a source
template. This dramatically reduces the time and effort required to
update all instrumentation setup.py files, and also reduces chances of
making manual mistakes.
@owais owais force-pushed the generate-setuppy branch from 538c3f1 to a310e01 Compare April 28, 2021 18:16
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Nice, this really does cleanup things and ensures consistency in the instrumentations.

@codeboten codeboten merged commit cb35cc4 into open-telemetry:main Apr 30, 2021
ocelotl pushed a commit to ocelotl/opentelemetry-python-contrib that referenced this pull request Apr 30, 2021
All instrumentations packages have almost exactly same setup.py files.
This commit adds a python script that generates it from a source
template. This dramatically reduces the time and effort required to
update all instrumentation setup.py files, and also reduces chances of
making manual mistakes.
ocelotl pushed a commit to ocelotl/opentelemetry-python-contrib that referenced this pull request May 13, 2021
All instrumentations packages have almost exactly same setup.py files.
This commit adds a python script that generates it from a source
template. This dramatically reduces the time and effort required to
update all instrumentation setup.py files, and also reduces chances of
making manual mistakes.
@owais owais deleted the generate-setuppy branch January 26, 2022 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants