-
Notifications
You must be signed in to change notification settings - Fork 119
Fix lakeview publish to default embed_credentials to false #4066
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
base: main
Are you sure you want to change the base?
Fix lakeview publish to default embed_credentials to false #4066
Conversation
|
hey @andrewnester |
andrewnester
left a comment
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.
Minor comment regarding tests but overall looks good! @denik could you TAL as well?
|
@andrewnester pushed the changes, |
|
Commit: 5dbf42a
11 interesting tests: 7 KNOWN, 2 flaky, 2 SKIP
Top 31 slowest tests (at least 2 minutes):
|
Head branch was pushed to by a user without write access
77a78a7 to
ec40595
Compare
ec40595 to
5dbf42a
Compare
|
An authorized user can trigger integration tests manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
|
@andrewnester there was a test failing on windows, fixed it, |
Fixes: #4001
Changes
Add override for
lakeview publishto explicitly sendembed_credentials: falsewhen--embed-credentialsflag is not provided.Why
The SDK omits
falseboolean values from JSON requests due toomitempty. This caused the API to default totrue, embedding credentials even when the flag wasn't set. The fix usesForceSendFieldsto always send the value explicitly.Tests
Added acceptance test verifying the request body contains
"embed_credentials": false.