Skip to content

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

Open
masomel wants to merge 13 commits into
masterfrom
auditor-server-cli
Open

[WIP] Implement the auditor-server application layer#193
masomel wants to merge 13 commits into
masterfrom
auditor-server-cli

Conversation

@masomel
Copy link
Copy Markdown
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
@masomel masomel force-pushed the auditor-server-cli branch from 7117f50 to be900a4 Compare November 9, 2017 22:52
Comment thread utils/binutils/encoding.go Outdated
}

// MarshalSTRToFile serializes the given STR to the given path.
func MarshalSTRToFile(str *protocol.DirSTR, path string) {
Copy link
Copy Markdown
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
Copy Markdown
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
Comment thread utils/binutils/logger.go Outdated
@@ -0,0 +1,134 @@
package binutils
Copy link
Copy Markdown
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
Copy Markdown
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.

Comment thread application/encoding.go Outdated
if err := response.Validate(); err != nil {
return &protocol.Response{
Error: protocol.ErrMalformedMessage,
// we don't want to return an ErrMalformedMessage
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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