Skip to content

Conversation

@madhavajay
Copy link

Firstly, I love this plugin ❤️.

However we are using tensorflow and related packages which are living in the dark ages of protobuf==3.19.x. I also noticed that this package doesn't use protobuf, but rather its upstream packages do, which begs the question, is it necessary to pin a specific protobuf version in this package?

codetiming==1.4.0
exceptiongroup==1.0.4
flake8==6.0.0
  - mccabe [required: >=0.7.0,<0.8.0, installed: 0.7.0]
  - pycodestyle [required: >=2.10.0,<2.11.0, installed: 2.10.0]
  - pyflakes [required: >=3.0.0,<3.1.0, installed: 3.0.1]
opentelemetry-instrumentation-digma==0.9.0
  - opentelemetry-exporter-otlp [required: ==1.12.0, installed: 1.12.0]
    - opentelemetry-exporter-otlp-proto-grpc [required: ==1.12.0, installed: 1.12.0]
      - backoff [required: >=1.10.0,<3.0.0, installed: 2.2.1]
      - googleapis-common-protos [required: ~=1.52, installed: 1.57.0]
        - protobuf [required: >=3.19.5,<5.0.0dev,!=4.21.5,!=4.21.4,!=4.21.3,!=4.21.2,!=4.21.1,!=3.20.1,!=3.20.0, installed: 3.20.3]
      - grpcio [required: >=1.0.0,<2.0.0, installed: 1.51.1]
      - opentelemetry-api [required: ~=1.3, installed: 1.12.0]
        - Deprecated [required: >=1.2.6, installed: 1.2.13]
          - wrapt [required: >=1.10,<2, installed: 1.14.1]
        - setuptools [required: >=16.0, installed: 65.6.3]
      - opentelemetry-proto [required: ==1.12.0, installed: 1.12.0]
        - protobuf [required: ~=3.13, installed: 3.20.3]
      - opentelemetry-sdk [required: ~=1.11, installed: 1.12.0]
        - opentelemetry-api [required: ==1.12.0, installed: 1.12.0]
          - Deprecated [required: >=1.2.6, installed: 1.2.13]
            - wrapt [required: >=1.10,<2, installed: 1.14.1]
          - setuptools [required: >=16.0, installed: 65.6.3]
        - opentelemetry-semantic-conventions [required: ==0.33b0, installed: 0.33b0]
        - setuptools [required: >=16.0, installed: 65.6.3]
        - typing-extensions [required: >=3.7.4, installed: 4.4.0]
    - opentelemetry-exporter-otlp-proto-http [required: ==1.12.0, installed: 1.12.0]
      - backoff [required: >=1.10.0,<3.0.0, installed: 2.2.1]
      - googleapis-common-protos [required: ~=1.52, installed: 1.57.0]
        - protobuf [required: >=3.19.5,<5.0.0dev,!=4.21.5,!=4.21.4,!=4.21.3,!=4.21.2,!=4.21.1,!=3.20.1,!=3.20.0, installed: 3.20.3]
      - opentelemetry-api [required: ~=1.3, installed: 1.12.0]
        - Deprecated [required: >=1.2.6, installed: 1.2.13]
          - wrapt [required: >=1.10,<2, installed: 1.14.1]
        - setuptools [required: >=16.0, installed: 65.6.3]
      - opentelemetry-proto [required: ==1.12.0, installed: 1.12.0]
        - protobuf [required: ~=3.13, installed: 3.20.3]
      - opentelemetry-sdk [required: ~=1.11, installed: 1.12.0]
        - opentelemetry-api [required: ==1.12.0, installed: 1.12.0]
          - Deprecated [required: >=1.2.6, installed: 1.2.13]
            - wrapt [required: >=1.10,<2, installed: 1.14.1]
          - setuptools [required: >=16.0, installed: 65.6.3]
        - opentelemetry-semantic-conventions [required: ==0.33b0, installed: 0.33b0]
        - setuptools [required: >=16.0, installed: 65.6.3]
        - typing-extensions [required: >=3.7.4, installed: 4.4.0]
      - requests [required: ~=2.7, installed: 2.28.1]
        - certifi [required: >=2017.4.17, installed: 2022.12.7]
        - charset-normalizer [required: >=2,<3, installed: 2.1.1]
        - idna [required: >=2.5,<4, installed: 3.4]
        - urllib3 [required: >=1.21.1,<1.27, installed: 1.26.13]
  - opentelemetry-sdk [required: ==1.12.0, installed: 1.12.0]
    - opentelemetry-api [required: ==1.12.0, installed: 1.12.0]
      - Deprecated [required: >=1.2.6, installed: 1.2.13]
        - wrapt [required: >=1.10,<2, installed: 1.14.1]
      - setuptools [required: >=16.0, installed: 65.6.3]
    - opentelemetry-semantic-conventions [required: ==0.33b0, installed: 0.33b0]
    - setuptools [required: >=16.0, installed: 65.6.3]
    - typing-extensions [required: >=3.7.4, installed: 4.4.0]
  - protobuf [required: ~=3.20, installed: 3.20.3]
pip==22.3.1
pipdeptree==2.3.3
pytest-asyncio==0.20.3
  - pytest [required: >=6.1.0, installed: 7.0.0]
    - attrs [required: >=19.2.0, installed: 22.1.0]
    - iniconfig [required: Any, installed: 1.1.1]
    - packaging [required: Any, installed: 22.0]
    - pluggy [required: >=0.12,<2.0, installed: 1.0.0]
    - py [required: >=1.8.2, installed: 1.11.0]
    - tomli [required: >=1.0.0, installed: 2.0.1]
wheel==0.38.4

I have also tested the newer version of opentelemetry-*==1.14.0 and the unit tests are passing. However its entirely possible there are some network features that need testing and may break compatibility.

I can change the PR to remove the protobuf dependency entirely, or not update opentelemetry if thats an issue.

Im mostly here for the awesome @ decorator!

madhavajay added a commit to OpenMined/PySyft that referenced this pull request Dec 9, 2022
@doppleware
Copy link
Contributor

Thanks for the PR! We will review with our sample documentations and test it works and hopefully merge soon

@amirkhonov
Copy link

@doppleware please expedite the review process. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants