opentelemetry-python icon indicating copy to clipboard operation
opentelemetry-python copied to clipboard

Add type hints to opentelemetry-sdk and run as a part of tests

Open srikanthccv opened this issue 4 years ago • 14 comments

Much of opentelemetry-sdk code doesn't have type hints. It would be great to add mypy type hints and run the tests similar to opentelemetry-api.

srikanthccv avatar Feb 14 '21 09:02 srikanthccv

Related to https://github.com/open-telemetry/opentelemetry-python/issues/773

codeboten avatar Feb 18 '21 16:02 codeboten

I've looked into this a few times and there were really annoying issues with namespace packages and finding the code. Mypy 0.8xx (#1575) has made huge improvements there and I think it should be much cleaner now. Like you mentioned, there is a lot of untyped code and code with types that was added as a best effort without running mypy. I think we will need a really relaxed config or start with only a few SDK files and slowly work up.

It would be awesome if we could get a nicer pattern for tox/mypy/setuptools so any new packages we scaffold will be fully checked from the start

aabmass avatar Feb 22 '21 22:02 aabmass

I recently updated mypy to 0.812 version (#1705). Finding code inside packages is much easier right now, so I can take care of running mypy tests for opentelemetry-sdk. Could you assign me to this issue?

dmolenda-sumo avatar Mar 31 '21 20:03 dmolenda-sumo

This issue was marked stale due to lack of activity. It will be closed in 30 days.

github-actions[bot] avatar May 02 '21 03:05 github-actions[bot]

@dmolenda-sumo Are you still working on this?

lzchen avatar May 03 '21 16:05 lzchen

Yes, I do. I've been a little busy recently, but I hope to finish this task soon

dmolenda-sumo avatar May 04 '21 08:05 dmolenda-sumo

This issue was marked stale due to lack of activity. It will be closed in 30 days.

github-actions[bot] avatar Jun 04 '21 04:06 github-actions[bot]

@dmolenda-sumo Any progress on this?

lzchen avatar Jun 04 '21 18:06 lzchen

It seems like Mypy is expecting there to be an __init__.py file in the opentelemetry directory

I've experimented with adding an empty __init__.py file to the root opentelemetry directory and it resolved all of the mypy typing issues that I was having with the opentelemetry libraries.

It seems like you would just need to add it to this location and everything would be fine: https://github.com/open-telemetry/opentelemetry-python/tree/main/opentelemetry-api/src/opentelemetry

aceberle avatar Mar 22 '22 19:03 aceberle

Actually, there appear to be quite a few __init__.py files missing

And I think I'm beginning to see that there might be a problem with the fact that the main module name 'opentelemetry' is used in multiple installation packages, when it probably would have made more sense to have a separate root module name for each package (i.e. 'opentelemetryapi' for -api, 'opentelemetrysdk' for -sdk, etc.)

aceberle avatar Mar 23 '22 00:03 aceberle

@aceberle This might help you https://mypy.readthedocs.io/en/stable/command_line.html#cmdoption-mypy-namespace-packages

srikanthccv avatar Mar 23 '22 00:03 srikanthccv

Hi @srikanthccv, thank you for directing me to that information. So is it expected that anyone pulling opentelemetry into their project will need to run mypy with this --namespace-packages argument to avoid the import errors?

I noticed that the tox.ini file uses this argument that you pointed out with the mypy command during the automated tests: https://github.com/open-telemetry/opentelemetry-python/blob/95aeeccb59945fee7f8cc40299df12a85ebc5783/tox.ini#L174

aceberle avatar Mar 23 '22 01:03 aceberle

Yes

srikanthccv avatar Mar 23 '22 01:03 srikanthccv

Thank you so much @srikanthccv, I added that to my configuration and now mypy is able to resolve all of the imports from opentelemetry-python!

aceberle avatar Mar 23 '22 12:03 aceberle

I"m happy to do a bit of work on this but to be honest I've found working with the opentelemetry repos very hard. I have plenty of experience working with complex setups but I keep finding issues with these repos, like linting seemingly not running properly in CI or errors simply following the CONTRIBUTING.md instructions. Would one of the maintainers that's familiar with the setup be willing to spend an hour on a call going through things and maybe we can make PRs to fix these sorts of issues or clarify docs where that make sense?

adriangb avatar Nov 16 '23 15:11 adriangb

This is one area we all agree that the current experience is really poor. I suspect, that even within the core team, we have found different ways of working around the problems. I want to see this improved. I spent some time in the past to replace our tooling in the hope of a better experience https://github.com/open-telemetry/opentelemetry-python/issues/3260 but that didn't materialize. I will be happy to get on a call but let me throw away my current setup and start fresh to see what are the common issues folks run into and help fix them.

srikanthccv avatar Nov 16 '23 19:11 srikanthccv

@adriangb @srikanthccv were you able to realize that call? @adriangb do you need further help?

ocelotl avatar Jan 15 '24 17:01 ocelotl

We did have a call, during it we found that formatting / linting wasn’t working at all (or wasn’t working for the sdk package, I don’t remember the exact details, maybe @srikanthccv does) so it was very productive!

That said this issues should remain open since the original request hasn’t been addressed. I personally would like to work on it but have not had time lately.

adriangb avatar Jan 15 '24 17:01 adriangb