-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
AVRO-3149: Add Rust based Implementation of Avro #1234
Conversation
This also adds support for namespaces and aliases
Use it instead of forcing EncodeAvro to infer a schema.
This should be faster than relying on format!. Avro requires floats to be encoded with little-endian. Almost all architectures encode f{32, 64} as little-endian: transmuting `self` directly instead of converting `to_bits().to_le()` because that looks prettier :D
Also some code cleanup
This should help automatic conversions with serde's (De)Serialize. And it's less ugly.
Instead of having bit spread across files.
With the upgrade to Rust 1.49, the clippy lint `unnecessary_cast` was [enhanced to work with integer and float literals][0]. There is one instance of this in the codebase that is breaking the build. This change removes the cast and fixes the clippy warning. [0]: https://github.com/rust-lang/rust-clippy/blob/00586dfdcd10c37cb8b132c72ed0558304955042/CHANGELOG.md#enhancements-1
* allow Value::Bytes to be assigned to Schema::Fixed as long as the length matches * also include encoding support Co-authored-by: Dave Grijalva <grijalva@gmail.com>
…es (#173) * Added ability to parse a list of schemas, which may have cross dependencies. Added tests * Fixing the formatting * A bit more formatting * Added a comma to please clippy * Switched an ok_or to an ok_or_else at the behest of clippy * Following fixes: Fixed some docstrings, a new error for name collisions when using parse_list, guarantees that input order matches output order * Removed an unused import * Small formatting fixes * Added another test involving fullnames * Added test that ensures when a recursive type is put in, the algorithm does blow the top off the stack but returns an error * removed unused variable * Updated readme and one tiny fix
This is necessary for resolving maps as records in unions
Ensure that symbol names are valid ([A-Za-z_][A-Za-z0-9_]*) and unique within an enum. Fixes #179
# specific language governing permissions and limitations | ||
# under the License. | ||
|
||
if [[ "$VIRTUAL_ENV" != "" ]]; then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't VIRTUAL_ENV
a python-specific environment variable? Does the rust build system work within a python ecosystem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a tool to automatically install and activate the virtualenv where pre-commit hooks live without having to do it manually. This for the quality of life of the developers, but it isn't necessary for the project to build at all. You can safely remove it, if you don't want to use it.
# specific language governing permissions and limitations | ||
# under the License. | ||
|
||
- repo: local |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should either put .pre-commit-config.yaml at the repo root, or ensure that it can apply different config files to different lang trees…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I would like to introduce pre-commit to lang/py as well, if not everything.)
|
||
.PHONY: clean-env | ||
clean-env: | ||
rm -rf $(VENV) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks dangerous. Can we be sure $(VENV) expands legitimately and doesn't contain globs?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Would git clean -xdf $(VENV)
be a good alternative?)
|
||
.PHONY: clean-hooks | ||
clean-hooks: | ||
rm -rf $(HOOKS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same concerns as regarding $(VENV)
|
||
.PHONY: clean-lint | ||
clean-lint: | ||
find . -type f -name *.rs.bk -delete |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could potentially use git clean
for this for better performance and safety.
I filled https://issues.apache.org/jira/browse/AVRO-3157 to address the two pending comments in this PR as well as to integrate the rust module into the docker build. I am going to merge this now so everyone can start working on it for both improvements and new features. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, I'm just a bit late but this seems like the right merge right now. It's going to take a bit more work to align it with the other languages, but keeping it consistent with the original will make the history more readable and obvious as changes arrive!
Thanks so much for the work everybody and @iemejia for shepherding these lines of code into place!
Make sure you have checked all steps below.
Jira
Tests
Commits
Documentation