Improve and fix support for compiling protobuf files#19
Merged
Conversation
Signed-off-by: Leandro Lucarella <leandro.lucarella@frequenz.com>
This can prevent some issues with editable installs and mypy, which sometimes complains about a frequenz.xxx module not having the py.typed file. Usually this file is created automatically by pip when doing proper installs, but when using editable installs that doesn't happen. Signed-off-by: Leandro Lucarella <leandro.lucarella@frequenz.com>
API repos are supposed to just ship python files generated from protocol buffer files. Only a stub __init__.py should be included, which doesn't really need to be checked. Allowing checks on the sources for an API repos triggers a lot of issues when the protocol buffers files are generated, as they don't pass any checks. An alternative would be to exclude auto-generated files, but it's not easy because each tool has its own exclusion mechanism, and some don't even have any (hello darglint), so it is simpler to just skip testing the stub __init__.py file. Signed-off-by: Leandro Lucarella <leandro.lucarella@frequenz.com>
To do this we declare a distutils entry point in the pyproject.toml file
to automatically add the `compile_proto` command using the
`CompileProto` proto class.
Then, in the `grpc_tools` module we just add the newly declared command
as a `build` sub-command, so it is executed first. This make all other
subsequent sub-commands will see the generated sources as if they were
already there.
The only downside of this is now we don't have an easy way to configure
the command. The options are there, and can be passed via command-line
(like:
```sh
python -c 'import setuptools; setuptools.setup()' compile_proto \
--proto-path=another/path
```
But there is no easy way to do it in a config file, or at least at the
moment I couldn't find a way to pass arbitrary options to setuptools
commands in `pyproject.toml`. If we need this, we could manually parse
the `pyproject.toml` file and look for options, just like
`setuptools_scm` does.
Signed-off-by: Leandro Lucarella <leandro.lucarella@frequenz.com>
Contributor
Author
|
Incidentally, while looking at the microgrid API repo to see how it was doing this, I realized the CI is not really testing anything. It uses |
Contributor
Author
|
With this I was able to migrate the dispatch API repo to use this one. |
Contributor
Author
|
OK, no complains for a month now, so force-merging 💪 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The main changes are:
Avoid running sessions on sources for API repos
API repos are supposed to just ship python files generated from protocol buffer files. Only a stub
__init__.pyshould be included, which doesn't really need to be checked.Allowing checks on the sources for an API repos triggers a lot of issues when the protocol buffers files are generated, as they don't pass any checks. An alternative would be to exclude auto-generated files, but it's not easy because each tool has its own exclusion mechanism, and some don't even have any (hello
darglint), so it is simpler to just skip testing the stub__init__.pyfile.Avoid the need to write a setup.py file for API repos
To do this we declare a distutils entry point in the
pyproject.tomlfile to automatically add thecompile_protocommand using theCompileProtoproto class.Then, in the
grpc_toolsmodule we just add the newly declared command as abuildsub-command, so it is executed first. This make all other subsequent sub-commands will see the generated sources as if they were already there.The only downside of this is now we don't have an easy way to configure the command. The options are there, and can be passed via command-line, like:
python -c 'import setuptools; setuptools.setup()' compile_proto \ --proto-path=another/pathBut there is no easy way to do it in a config file, or at least at the moment I couldn't find a way to pass arbitrary options to setuptools commands in
pyproject.toml. If we need this, we could manually parse thepyproject.tomlfile and look for options, just likesetuptools_scmdoes.