-
Notifications
You must be signed in to change notification settings - Fork 50
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
Improve unmarshalling for binary payloads #3826
Conversation
c3c532c
to
82db788
Compare
82db788
to
c5efc6b
Compare
Breaking ChangesNo Breaking Changes were found 👍 |
Summary of ChangesNo Breaking or Non-Breaking Changes were found 👍 |
New Resource ID Segments containing Static IdentifiersNo new Resource ID Segments containing Static Identifiers were identified in the set of changes 🤙. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one minor comment around LOG_LEVEL
but this otherwise LGTM 👍
@@ -59,6 +59,10 @@ func (c ServeCommand) Run(args []string) int { | |||
serviceNames = pointer.To(strings.Split(serviceNamesRaw, ",")) | |||
} | |||
|
|||
if logLevel := strings.TrimSpace(os.Getenv("PANDORA_LOG")); logLevel != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not made the change to the Data API yet, but the other tools are now using LOG_LEVEL
fwiw, probably worth doing that here too:
if logLevel := strings.TrimSpace(os.Getenv("PANDORA_LOG")); logLevel != "" { | |
if logLevel := strings.TrimSpace(os.Getenv("LOG_LEVEL")); logLevel != "" { |
(in the other tools this is done in main.go
- but that's a minor nit we can clear up later if needed)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah cool, I will align it with the rest of the tooling 👍
Breaking ChangesNo Breaking Changes were found 👍 |
Summary of ChangesNo Breaking or Non-Breaking Changes were found 👍 |
New Resource ID Segments containing Static IdentifiersNo new Resource ID Segments containing Static Identifiers were identified in the set of changes 🤙. |
This removes the double pointer workaround for unmarshaling binary payloads into a
[]byte
.Needs coordination with corresponding base layer PR: hashicorp/go-azure-sdk#890