-
Notifications
You must be signed in to change notification settings - Fork 0
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
Issue48 49 fix hmac signing #50
Conversation
@graft ☝️ ? |
Hmm, could you comment on my |
Sorry, I missed your question about The problem with that was more of a "niceness" / UX issue. When Metis sends back error messages to Magma, they are JSON errors. But because that code used to be
you would get something a little less user-friendly (and Ruby-specific), like
|
We should definitely squash those ugly ruby errors. However, here you are in client.rb, receiving the error, so why should you turn it into JSON here? If the problem is it needs to be transmitted on from here (i.e., magma uses etna::client to talk to metis, gets an error, and wants to pass it on to the user) that should be solved outside of the client context (magma is another service generating errors and should properly format them there). |
Sorry, maybe I'm misunderstanding your question.... That method call doesn't turn the error into a JSON array, it turns the errors into a JSON-string so they can be transmitted cleanly via Or, are you suggesting that Etna Client should pass a JSON object directly back to Magma, outside of raising an Error? We could also just send back |
Okay, I misunderstood what was happening here - eventually Etna::Client is packaging whatever error it gets as an Etna::Error, which can't hold a json object. This seems an unfortunate weakness in the Etna::Error type, so your solution is the only way to actually pass a json error out of the Etna::Client. However it might be nicer to allow the Etna::Error to attach a json object instead of (or in addition to) just a string message, which would let you pass it out of here without having to JSONify it (and unpack it in Magma). Then Magma can take the JSON off Etna::Error directly and do whatever it wants. |
Let's 🚢 here and punt the Etna::Error improvements to a new issue. |
Close #48
Close #49
Per our conversation earlier today, these small tweaks seem to make the Etna Client able to sign request bodies.