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

[WIP] Implement the auditor-server application layer #193

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

masomel
Copy link
Member

@masomel masomel commented Nov 2, 2017

Part of #151

@masomel masomel added this to the 0.2.0 milestone Nov 2, 2017
@masomel masomel self-assigned this Nov 2, 2017
}

// MarshalSTRToFile serializes the given STR to the given path.
func MarshalSTRToFile(str *protocol.DirSTR, path string) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related: #137 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely. This particular function is just for the test implementation, but in implementing the auditor, having a persistent storage backend is becoming more and more necessary. We should get back to implementing that soon.

@masomel masomel force-pushed the auditor-server-cli branch 2 times, most recently from 1ab0a2a to df1762c Compare December 20, 2017 23:39
@@ -0,0 +1,134 @@
package binutils
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have application/logger.go.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ugh, thanks for catching that. I thought I removed the binutils package when I rebased.

@@ -77,8 +77,12 @@ func UnmarshalResponse(t int, msg []byte) *protocol.Response {
Error: res.Error,
}
if err := response.Validate(); err != nil {
return &protocol.Response{
Error: protocol.ErrMalformedMessage,
// we don't want to return an ErrMalformedMessage
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

@masomel masomel Jan 15, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the problem is that this function was still returning an ErrMalformedMessage even if the error is in errors. In other words, because Validate() in message.go returns msg.Error, which gives err == nil after Validate() returns, the return &protocol.Response{Error: protocol.ErrMalformedMessage } statement was always being called, even when the msg.Error was in errors.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I gave it a try in https://github.com/coniks-sys/coniks-go/compare/unmarshalling. If it's ok, feel free to merge to this branch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those changes looks good to me. Thank you! I also like that you re-wrote the test cases. I'll merge this branch when I get a chance.

@masomel masomel removed the blocked label Feb 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants