iovisor / bcc

BCC - Tools for BPF-based Linux IO analysis, networking, monitoring, and more
Apache License 2.0
20.51k stars 3.88k forks source link

Local dependency allowed when contributing to `tools/`? #3362

Open yzgyyang opened 3 years ago

yzgyyang commented 3 years ago

Hi there!

We are developing some NFS-related scripts similar to those in bcc tools/, which we'd really like to contribute back to this repo. However, we used OOP design and split a lot of shared functions into a parent class that handles multiple formats of output, initializing and cleaning the map, etc, which lives in another file and used by all scripts.

I am wondering, any chance this "local dependency" will be accepted, or do we have to submit standalone scripts (which would result in some refactoring and code duplication)?

yzgyyang commented 3 years ago

Our current structure looks like:

.
├── lib
│   ├── lib
│   │   ├── shared.py
├── tools
│   ├── script1.py <- depend on ../lib/lib/shared.py
│   ├── script2.py <- depend on ../lib/lib/shared.py
│   ├── ...
yonghong-song commented 3 years ago

Thanks @yzgyyang If shared.py is truely common and can be reused by other [future] tools, we can put the script under src/python/bcc/ directory, e.g., we have tcp.py there for some common tcp functionalities. Looks like you have multiple tools script, I guess the reason is probably trying to do one thing per script, this is okay. we can check how useful it could be for broader community.

yzgyyang commented 3 years ago

@yonghong-song I just opened a PR #3367 with the code pulled from our repo as-is - the shared class is mainly for initializing a top-like script, handle different outputs, etc. I wanted to get an idea of if this is an acceptable way, or if we need to do some more refactoring for it to land. Thanks!