Skip to content
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

fix AVRO-1901 - record named "Exception" generated bad code #115

Closed

Conversation

radai-rosenblatt
Copy link
Contributor

catch fully qualified java.lang.Exception in record template

@nielsbasjes
Copy link
Contributor

Your test only applies to the Java implementation.
I think the test schema entries should be moved to https://github.com/apache/avro/blob/master/share/test/schemas/specialtypes.avdl which was created specifically for these kinds of special problem cases across languages.

I would not be surprised if other languages have the same problem.

@radai-rosenblatt
Copy link
Contributor Author

@nielsbasjes - I've reworked the patch and moved the schema entry to specialtypes.avdl as you suggested

@rdblue
Copy link
Contributor

rdblue commented Sep 4, 2016

This looks good to me. @nielsbasjes, do you have anything else or do you think this is ready to commit?

@nielsbasjes
Copy link
Contributor

The change itself looks good to me.
Both the baseball/Player.java files still need to be updated (otherwise the tests fails)
With those changes it passes all languages.

One strange thing I noticed when doing "git log" was that the commit message starts with a funny (invisible) character.

commit 75e0a0ce4ed21d74cfe26ef4ba97ff051297ce0e
Author: radai-rosenblatt <radai.rosenblatt@gmail.com>
Date:   Tue Aug 23 06:52:14 2016 -0700

    <U+0096>AVRO-1901 - fix java record template to allow generatic a class called Exception

With those minor things fixed it's a +1 from me.

@radai-rosenblatt radai-rosenblatt force-pushed the bad-record-names branch 2 times, most recently from 4ad9f91 to 65d1372 Compare September 5, 2016 22:40
Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
Signed-off-by: radai-rosenblatt <radai.rosenblatt@gmail.com>
@radai-rosenblatt
Copy link
Contributor Author

radai-rosenblatt commented Sep 6, 2016

@nielsbasjes PR has been reworked into 3 commits - the 1st is my actual fix (with the update to both baseball/Player.java files and the msg fixed). the other 2 are fixes to unrelated issues that were preventing a successful build. currently my build runs and passes completely on linux.

@nielsbasjes
Copy link
Contributor

Thanks.
I squashed and merged your changes and then ran all tests for all languages.
So also +1 from me.
I just committed this.

@nielsbasjes
Copy link
Contributor

@radai-rosenblatt : Can you please close this pull request? Thanks.

@radai-rosenblatt
Copy link
Contributor Author

thank you

@radai-rosenblatt radai-rosenblatt deleted the bad-record-names branch February 22, 2019 00:54
iemejia pushed a commit that referenced this pull request Jun 11, 2021
* Move from failure to thiserror

Closes #115

This is still a WIP branch, with lots of TODOs and some things about
thiserror I still can't wrap my head around. However, the heavy-lifting
is done, the failure crate has been removed from the list of
dependencies and compilation, tests, benchmarks and linting are all
green.

The two biggest things I have yet to figure out are:
1. How to deal with the errors manually defined in ser.rs and de.rs:
   they are publicly available and as soon as I touch anything I hit
   cryptic serde errors
2. How to make errors like TryFromIntError part of more abstract ones
   like ParseSchemaError, which could have a source error or just a
   String description.

* Update tests/io.rs

Co-authored-by: Joel Höner <joel@zyantific.com>

* Renaming errors + apply clippy consistently

 Rename AvroError to Error
 Removed redundant Error suffix from variants
 Introduce AvroResult shorthand alias with crate visibility
 Align clippy invocation in tests with the one in pre-commits

* Stop stressing about generic errors and add a couple more sprecific ones

* Centralize Ser and De errors into Error

The trick was implementing the ser::Error and de::Error trait for
crate::errors::Error and return Error::Ser and Error::De variants
in the implementation of the custom() method.

* SnappyCdcError as struct for consistency

* Update CHANGELOG

* Update CHANGELOG, README and add a Migration Guide page

Co-authored-by: Joel Höner <joel@zyantific.com>
SingingBush pushed a commit to SingingBush/avro that referenced this pull request Jun 29, 2023
martin-g pushed a commit that referenced this pull request May 20, 2024
…in (#2310)

AVRO-3732: 

* Try again to avoid gradle welcome banner in CI

* ci: try turning on github actions compatibility tests

* fix ci syntax

* ci: fix yaml indentation

* ci: comment out fail fast

* ci: fix matrix structure

* CI: try matrix with allowed failures

* CI: give up on conditional failures allowed for now; exclude java 13

* CI: info output for builds

* Try to resolve the test failures on windows regarding default encoding handling

* Fix encoding support on windows, for real this time

* CI: add unsupported-java-versions job

* CI: run the unsupported java versions job on all the OS versions

After all, they'll all fail almost immediately anyway

* README: Update badge to use github actions rather than travis

* add support for generating optional getters

* README: fix CI badge syntax

* README: fix CI badge image

* add doc for optional getter field generation

* Update changelog to note the recent merged pull request

* CI: disable the gradle daemon to try to eliminate the sporadic clean failure on windows

* Remove security policy; not a CommerceHub OSS project any more

* Update various files for commercehub-oss -> davidmc24 github move

* Working version of custom conversions against modern gradle; still need to adjust for earlier versions

* Don't use MapProperty yet; it wasn't introduced until Gradle 5.1

* Don't use ListProperty

It changed incompatibly between Gradle 4.4 and Gralde 4.5

* Don't use Class.newInstance(), as it was deprecated in Java 11

* Update issue templates

* Update bug_report.md to add a checklist

* Update feature_request.md to include a checklist

* CI: don't bother doing maintenance builds on old OS versions; only use latest

* CI: update another place with os versions that I missed

* version: 0.18.0

* version: 0.18.1-SNAPSHOT

* update to Avro 1.9.2 since https://issues.apache.org/jira/browse/AVRO-2548 has been fixed there

* Add support for Gradle 6.0-6.2, drop support for gradle <5.1 (#101)

* Update changelog for #104

* Add support for Java 13

* Add support for testing multiple kotlin versions

* Update plugin's own build to address some deprecation warnings of APIs being removed in Gradle 7

* BuildCacheSupportFunctionalSpec no longer needs an @IgnoreIf, as we only support versions where the Build Cache is supported.

* Remove license plugin

It was resulting in deprecation warnings about Gradle 7, a new version wasn't available, and I don't think it was providing real value.

* Lots of test updates

* Remove taskInfoAbsent, as it isn't needed any more with the versions of Gradle we support
* Remove isMultipleClassDirectoriesUsed, as all versions of Gradle we support now use it
* Leverage GradleRunner's withPluginClasspath feature when able to use plugin DSL to apply the test plugin
* Add addDefaultRepository utility method
* Add applyPlugin override that takes a version
* Rename addDependency to addImplementationDependency; add addRuntimeDependency and addDependency that takes a configuration argument
* Use stripMargin more consistently

* Add tests for Kotlin DSL usage (#61)

* Handle a test that appears to fail on Windows due to weird file locking behaviors

* Update to note a Kotlin-Java version incompatibility

* Update to gradle 6.2.2

* Official Gradle Wrapper Validation Action

See: https://github.com/gradle/wrapper-validation-action
Added as a dedicated Workflow

* Support Task Configuration Avoidance (#97)

https://docs.gradle.org/current/userguide/task_configuration_avoidance.html

Thanks to [dcabasson](https://github.com/dcabasson) for the collaboration

* Update test result directory names

* Work around a bug showing in Gradle 5.1

It appears that in Gradle 5.1, TaskContainer's `withType` overwrites the results of `matching`, causing java compilation tasks to be returned.
This results in a circular task dependency.
Changing the order to filter by type first fixes it.

* See if we can get Java 14 support working with a Gradle 6.3 nightly build

* Update codenarc support so it works in Java 14+; update compatibility notes

* version: 0.19.0

* version: 0.19.1-SNAPSHOT

* Create FUNDING.yml

* Update bug_report.md

* Fix schema dependency resolution when types are referenced with a `{ "type": NAME }` block rather than just `NAME` (#107)

* Eliminate `NullPointerException` handling in schema dependency resolution, as it no longer appears to be needed.

* version: 0.19.1

* version: 0.19.2-SNAPSHOT

* Add  (#115)

* Update version compatibility support

* Built using Gradle 6.5
* Updated compatibility testing to include Java 14
* Updated compatibility testing through Gradle 6.5

* version: 0.20.0

* version: 0.20.1-SNAPSHOT

* ci: Tweak how the jobs are run to reduce execution time and unreliability

Mac OS X jobs have been failing with messages that they were cancelled.

My theory is that either Mac OS X jobs are flakey right now, or I'm hitting an execution timeout of some kind.

Either way, this reduces how long jobs run for and how many are run on Mac OS X.
Hopefully it will complete faster (providing faster feedback) and be more reliable.

* CI: try using eskatos/gradle-command-action to improve build speed

This enables use of the daemon and easy caching of wrapper
Might enable depenendcy caching latter (isn't on by default yet)

* CI: try turning on build scans

* CI: fix the build

It looks like there may be an incompatibility between gradle-entrprise and gradle-github-actions; try just gradle-enterprise for now.
Also, gralde-enterprise needs to be applied in the settings file with the latest version.

* CI: some build tweaks for performance

Enable github actions plugin to capture metadata in build scans
Enable parallel builds
Enable dependency caching

* Correcting cache annotation for classpath

* Correcting cache annotation for classpath - add items to unreleased changelog

* Update to Avro 1.10.0

* Add a test project to aid in reproducing problems

* Test cleanup and CHANGES.md additions for Avro 1.10.0 support

* Fix test name for optionalGettersForNullableFieldsOnly

* Confirm that the nullable getter is still generated when createOptionalGetters is enabled

* Formatting fixes and test simplification

* Fix optionalGettersForNullableFieldsOnly test naming and CHANGES formatting

* Ensure option conditions are checked against relevant file portions

* Remove apache staging dependency and pull release from JCenter

* Ensure createOptionalGetters test checks correct mainClassContent

* Remove extraneous newline from test repository definition

* version: 0.21.0

* version: 0.21.1-SNAPSHOT

* build: add coverage reporting using JaCoCo and Codecov

* CI: flag codecov report by OS

* Add support for multiple IDL files with the same name in different directories (#123)

* Update AvroUtils and Strings, with unit test coverage

* Fix codenarc failures

* Fix a test typo (Fixes #125)

* Update test-project to remove references to dataTimeLogicalType

* Plugin DSL is no longer incubating, and is now recommended.

* Upgrade to Avro 0.10.1 because of https://issues.apache.org/jira/browse/AVRO-2924

* Added compatibility testing against 1.10.0 and 1.10.1

* More spock assert about generated Java Code was added. Type uuid is not compilable with Avro 1.10.1

* Avro 1.10.1 was set also in test-project/build.gradle

* Minor post pull-request-merge tweaks

* Update compatibility with Java/Gradle, tweak build spead

* Improve Java version support

Use a Java 15-compatible version of Jacoco (currently requires a snapshot build)
Use a new Java 9+ option in Gradle 6.6 that use cross-compilation to ensure that APIs not available in previous versions aren't used improperly

* Correct Java 15 Gradle version compatibility

* Add support for configuration cache and update kotlin testing

Closes #129

* Make codenarc happy

Fix ClosureStatementOnOpeningLineOfMultipleLineClosure violation

* Address not all versions of kotlin plugin supporting config cache

* version: 0.22.0

* version: 0.23.0-SNAPSHOT

* Add link to discussions for Q&A

* Revert "Add link to discussions for Q&A"

This reverts commit 946cf47.

* Add link to discussions for Q&A

* Add feature requests to template chooser

* Try moving feature requests to discussions

* Update README to reflect changes in historical version handling

* Update notes about pre-1.0 versions

* Work towards a 1.0 release

* Fix up POMs, add signing; to satisfy Maven Central requirements

* version: 1.0.0

* version: 1.0.1-SNAPSHOT

* version: 1.0.0 (take 2)

* version: 1.0.0 (take 3)

* version: 1.0.1-SNAPSHOT

* Remove remaining references to jcenter

* Build using Avro 1.10.2

* Build using Gradle 6.8.3

* Try running compatibility tests from github actions rather than gradle

* Try extracting kotlin plugin compatibility testing to its own workflow

* Try to fix CI job

* Try to fix CI job

* Fix testKotlinPluginCompatibility

* Next try at kotlin compatibility

* More matrix stuff

* More matrix compatibility tweaks

* More matrix compatibility tweaks

* More matrix compatibility tweaks

* More matrix compatibility tweaks

* Upgrade gradle used to build the project to 7.0-rc-1 in order to support the build running on Java 16

* More changes for Gradle 7 compatibility

* Add gradle compatibility tests

* More spock 2.0 and Gradle 7 compatibility changes

* Fix checkstyle violation

* Tweak CI targets

* Adjust compatibility notes/testing for java 16/17

* Use nexus publish plugin to automate management of OSSRH interactions

* Adjust test report directories

* Fix os compatibility tests

* Update changelog

* version: 1.1.0

* version: 1.1.1-SNAPSHOT

* Try to fix publishing

* Temporary change to test the CI publishing fix

* Revert "Temporary change to test the CI publishing fix"

This reverts commit 4fb85a4.

* Re-add support for Avro 1.9.x

* Update release notes

* Throw error if avdl will be overwritten

* Add to changelog

* Minor touchups on avpr conflict detection

* Add support for better coverage reporting... and disable it because it conflicts with configuration caching in Gradle 7.0

* version: 1.2.0

* version: 1.2.1-SNAPSHOT

* Add examples/default-custom-types

* Update for Gradle 7.1

* CI: update to setup-java v2

* Update comment on code coverage support

* Update build/test to Gradle 7.1.1

* Update test-project to reproduce #167

* Explicitly declare dependencies between sources jar tasks and GenerateAvroJavaTasks (#167)

* Satisfy checkstyle

* Simplify sources jar handling and fix configuration avoidance

* Fix compatibility with Gradle before 6.0

* version: 1.2.1

* version: 1.2.2-SNAPSHOT

* Upgrade to Gradle 7.2

* Update CI to indicate that Java 17 has been released, even though it's not supported yet.

* Upgrade CodeNarc to build cleanly on Java 17

* Add some scripts to aid troubleshooting

* Update readme to use implementation rather than compile configuration

* Add avsc-from-external-jar and avsc-from-subproject examples

* Make some minor adjustments to GradleAvroProtocolTask due to investigation of #174

* Add design for potential "additional schema" configurations feature

* Add block to example to show custom resource dir usage

* Update avsc-from-subproject example to compile schema for schema project, exclude generated classes from cat jar

* Fix cachability for cat jar in example

* GenerateAvroProtocolTask: don't delegate to the system classloader, even implicitly

* Add an example that generated a UUID field

* Add test project for kotlin

* Update for avro 1.11.0

* Drop support for Avro 1.9.0-1.10.2

Due to an incompatibility introduced in Avro 1.11.0

error: no suitable constructor found for SpecificRecordBuilderBase(Schema,SpecificData)

* Update kotlin compatibility testing

* Update Java compatibility testing

* Update to build with Gradle 7.3; add Java 17 compatibility

* CI: Attempt to fix kotlin compatibility testing

* More kotlin compatibility testing fixes

* CI: update to gradle-build-action v2

* version: 1.3.0

* version: 1.3.1-SNAPSHOT

* CI: enable automatic staging repository releasing

* Drop kotlin plugin integration

* Add note about seeking new maintainer to readme

* version: 1.4.0

* version: 1.4.1-SNAPSHOT

* Add 'additionalVelocityTool' capability. Allows user to provide tools that will be available in Velocity templates at generation time.

* Update changelog

* Update versions of Gradle/Avro/Java

* version: 1.5.0

* version: 1.5.1-SNAPSHOT

* Support specifying classpath for additional velocity tool classes

* Add test for classpath property in GenerateAvroJavaTask

* Minor cleanup

* Fix test: escape backslashes in a Windows path

* Drop Java compatibility testing for outdated versions

* Update for Gradle 7.6 and Java 19

* Fix up cross-version compatibility for the Gradle 7.6 adjustments

* Update changelog

* version: 1.6.0

* version: 1.6.1-SNAPSHOT

* Update changelog

* Add ability to use conversions and type factories residing outside of build classpath

* Fix compatibility with Gradle < 7.1

* Add documentation for using conversions and type factories located outside of build classpath

* Add deprecations for methods used to configure conversions and type factories with classes

* Prep for 1.7.0 release

* version: 1.7.1-SNAPSHOT

* Fix vulnerabilities in transitive dependencies

* version: 1.7.1

* version: 1.7.2-SNAPSHOT

* Update github actions that ran in node 12

* Fix Kotlin DSL setup snippet

* move everything to lang/java/gradle-plugin

---------

Co-authored-by: David M. Carr <david@carrclan.us>
Co-authored-by: David M. Carr <dcarr@commercehub.com>
Co-authored-by: Ben Speakmon <benjamin.speakmon@nordstrom.com>
Co-authored-by: Markus Helbig <mhelbig81@googlemail.com>
Co-authored-by: Denis Cabasson <denis_c@apple.com>
Co-authored-by: mcwhitak <mcwhitaker@fastmail.com>
Co-authored-by: Michael Whitaker <michael.whitaker@nordstrom.com>
Co-authored-by: Vladimir Kralik <vkralik@gmail.com>
Co-authored-by: Ola Hungerford <Ola.Hungerford@nordstrom.com>
Co-authored-by: David M. Carr <dcarr@gradle.com>
Co-authored-by: Dave Cracauer <david.cracauer@bestbuy.com>
Co-authored-by: Paul Kofmann <crtlib@gmail.com>
Co-authored-by: Marcin Erdmann <erdi84@gmail.com>
Co-authored-by: Marcel Henrich <marcel.henrich-extern@deutschebahn.com>
Co-authored-by: Antonio Martinović <antonio.martinovic@pm.me>
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