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

Field names don't allow fully qualified names with periods. #8

Closed
arunmk opened this issue May 1, 2015 · 6 comments
Closed

Field names don't allow fully qualified names with periods. #8

arunmk opened this issue May 1, 2015 · 6 comments
Assignees
Labels

Comments

@arunmk
Copy link

arunmk commented May 1, 2015

When the example code server.go in linkedin/goavro/examples/net is run and the client (built from client.go) in the same directory is run, the server program crashes. This is because the timestamp field is set using the fully-qualified path in server.go:

98         // you can fully qualify the field name
99         someRecord.Set("com.example.timestamp", int64(1082196484))

I have a simple patch:

commit a0dd9adc255b38b733f66c0e8ce3f038eae5b2b2
Author: Arun M. Krishnakumar <xxxxxx@gmail.com>
Date:   Fri May 1 10:53:21 2015 -0700

    Allow fully qualified paths in field names.

diff --git a/name.go b/name.go
index bf32798..bd4efcb 100644
--- a/name.go
+++ b/name.go
@@ -85,7 +85,7 @@ func (e ErrInvalidName) Error() string {
 }

 func isRuneInvalidForFirstCharacter(r rune) bool {
-       if (r >= 'A' && r <= 'Z') || (r >= 'a' && r <= 'z') || r == '_' {
+       if (r >= 'A' && r <= 'Z') || (r >= 'a' && r <= 'z') || r == '_' || r == '.' {
                return false
        }
        return true
@@ -106,7 +106,7 @@ func checkName(s string) error {
                return &ErrInvalidName{"start with [A-Za-z_]"}
        }
        if strings.IndexFunc(s[1:], isRuneInvalidForOtherCharacters) != -1 {
-               return &ErrInvalidName{"have second and remaining characters contain only [A-Za-z0-9_]"}
+               return &ErrInvalidName{"have second and remaining characters contain only [A-Za-z0-9._]"}
        }
        return nil
 }

If you think the above is valid and complete, I can create a pull request that can be merged, or you could take the patch yourself.

@arunmk arunmk changed the title Field names don't seem to allow fully qualified names with periods. Field names don't allow fully qualified names with periods. May 1, 2015
@arunlw
Copy link

arunlw commented May 4, 2015

CC @karrick

@karrick
Copy link
Contributor

karrick commented Jul 7, 2015

Sorry folks. I have not abandoned this project, but my full time job has kept me pretty busy.

I noticed that namespaces are not implemented correctly. I started working on a fix. I'll take a look at the above patch when I get a minute. Thank you for your feedback!

@karrick karrick self-assigned this Jul 7, 2015
@karrick karrick added the bug label Jul 7, 2015
@c0b
Copy link

c0b commented Feb 21, 2017

ping @karrick just wonder if you or any other one at linkedin have a minute in 2017? just scan over current open issues

@karrick
Copy link
Contributor

karrick commented Jun 1, 2017

@c0b,

Please check out the newly rewritten version of this library at https://github.com/karrick/goavro

It properly handles records with fully qualified field names.

@karrick karrick closed this as completed Jun 1, 2017
@c0b
Copy link

c0b commented Jun 1, 2017

thanks for the update, that definitely blew away all the doubts on whether the project is abandoned.

On the API breaking changes for go libraries, I see the gopkg service is pretty popular, (like https://gopkg.in/yaml.v2) would you like also make a pointer there https://gopkg.in/goavro.v2 to your personal repo?

@karrick
Copy link
Contributor

karrick commented Jun 1, 2017 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants