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.

Sign up for GitHub

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

Jump to bottom

[azdatalake][STG84] Append with flush #22245

Merged
merged 34 commits into from
Jan 17, 2024
Merged

[azdatalake][STG84] Append with flush #22245

merged 34 commits into from
Jan 17, 2024

Conversation

tanyasethi-msft
Copy link
Member

  • The purpose of this PR is explained in this or a referenced issue.
  • The PR does not update generated files.
  • Tests are included and/or updated for code changes.
  • Updates to module CHANGELOG.md are included.
  • MIT license headers are included in each file.

RickWinter and others added 30 commits January 4, 2024 09:46
@RickWinter @chlowell
Update package versions (#22201)
d59677e 
* Update package versions

* upgrade to MSAL v1.2.1

* add token_type to fake tokens

* revert debugging

---------

Co-authored-by: Charles Lowell <10964656+chlowell@users.noreply.github.com>
@azure-sdk
Increment package version after release of messaging/azeventgrid (#22060
4228971 
)
@azure-sdk
Increment package version after release of ai/azopenai (#22126)
8c72917
@souravgupta-msft
azblob: STG91 upgrade (#22194)
5798b0d
@souravgupta-msft
azfile: STG91 upgrade (#22193)
7f7c19b
@fkarg
update source link to new azopenai location (#22208)
3ba6817 
* link current instead of depricated path

* remove newline at end of file
@jhendrixMSFT
More azappconfig cleanup (#22209)
4162eea 
* More azappconfig cleanup

Update BeginCreateSnapshot to return the complete Snapshot type.
Remove unused response types.
Added some missed error checking in a few tests.

* fix a few doc comments
@azure-sdk @antkmsft
Fix spelling (preceeding=> preceding) (#22202)
2d017b7 
Co-authored-by: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com>
@souravgupta-msft
Storage: Reject HTTP endpoints (#22183)
06a9327
@richardpark-msft
[azeventhubs] Fixing an issue where a processor wouldn't grab partiti…
e63fa24 
…ons (#22153)

The processor load balancer had a bug where a processor would not claim partitions even if it didn't yet have the proper share.

Added in stress tests (balance, multibalance) to run several scenarios with our supported strategies and different parallel owners.

Fixes #22097
@jhendrixMSFT
Clean up ETags in azappconfig (#22212)
9f9219c 
Use azcore.ETag instead of string for public surface area.
Add doc comments for non-standard ETag option names.
@richardpark-msft
[azopenai] Fixing issue where you can't use whisper with m4a files. (#…
ecdb3a6 
…22210)

Fixing issue where you can't use whisper with m4a files.

* It's one of the formats that doesn't seem to be recognized without an explicit file extension, which you can pass via Filename
* My tests were too heavily dependent on implementation details of the models. Changing this out to check that things are working correctly without checking the exact contents of the response.
* Also, rerecorded tests since we're doing multiple audio tests as well.

Fixes #22195
@souravgupta-msft
Updating changelog for STG91 preview (#22216)
ed7cbbe
@souravgupta-msft
azfile: STG 91 Preview (#22217)
a672b5d
@azure-sdk
Increment package version after release of storage/azblob (#22219)
a5c4497
@richardpark-msft
[azopenai] Update changelog for release. (#22215)
c7abe2d
@jhendrixMSFT
Replace string with azcore.ETag (#22218)
f386581 
Missed one in last round of clean-up.
@jhendrixMSFT
Add script to go mod tidy modules (#22214)
dbf4d0d 
Will tidy all modules under the current working directory or under the
specified directory.
@azure-sdk
Increment package version after release of storage/azfile (#22220)
8b19787
@kushagraThapar
Added Simon and Kushagra as code owners on GO Cosmos SDK (#22221)
bdf62de
@richardpark-msft
[azservicebus] Updating batching to allow for a configurable wait time (
46b3614 
#22154)

Updating batching to allow for a configurable wait time. Can lead to fuller batches for people that want to tune it.

Fixes #19172
@azure-sdk @scbedd
Sync eng/common directory with azure-sdk-tools for PR 7459 (#22222)
0fbeb23 
* check for the presence of a compatible powershell. ensure that we always return a list of tags

* allow the script to require pshell6+

* remove the en-us from the link

---------

Co-authored-by: Scott Beddall (from Dev Box) <scbedd@microsoft.com>
@azure-sdk @lmolkova
Fix image tag for multiimage deployments (#22224)
58413e0 
Co-authored-by: Liudmila Molkova <limolkova@microsoft.com>
@tadelesh
Refresh azcontainerregistry module (#22161)
e274f37 
* upgrade to latest codegen

* merge resolver

* enable span and add changelog

* update module name
@Alancere
fix generator tool (#22205)
fbe326d 
* fix generator tool

* gofmt

* fix

* goimports
@tanyasethi-msft
azdatalake preview stg82
3264125
@tanyasethi-msft
azdatalake preview stg82 (#22227)
467afcf
@chlowell
Rename azidentity/cache pipeline (#22231)
4b274bc
@benbp
Handle errors in mod tidy script (#22234)
c99ab58
@tanyasethi-msft
Merge branch 'Azure:main' into main
498b32e
@github-actions github-actions bot added the Storage Storage Service (Queues, Blobs, Files) label Jan 12, 2024
jhendrixMSFT
jhendrixMSFT reviewed Jan 12, 2024
View reviewed changes
sdk/storage/azdatalake/file/models.go
@@ -243,6 +243,8 @@ type AppendDataOptions struct {
LeaseAccessConditions *LeaseAccessConditions
// CPKInfo contains optional parameters to perform encryption using customer-provided key.
CPKInfo *CPKInfo
//Flush Optional. If true, the file will be flushed after append.
Flush *bool
Copy link
Member

Choose a reason for hiding this comment

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

If Flush: nil is semantically equivalent to Flush: false then it would be better to define this as Flush: bool as that's easier to work with.

Copy link
Member Author

Choose a reason for hiding this comment

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

We usually use pointers for optional boolean parameters, like here. Also, this Flush parameter is defined as a pointer in generated code.
Happy to discuss if a change is needed in this approach.

Copy link
Member

Choose a reason for hiding this comment

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

Agree "true" looks better and more user friendly then "to.Ptr(true)". Are there other places where we send true/false in form of pointers ?

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 that auto generated code itself take *bool and hence our wrappers have also taken dependency on similar types.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes @vibhansa-msft, everywhere we send true/false in form of pointers, eg - Link

Copy link
Member

Choose a reason for hiding this comment

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

There is a check where if flush is nil, it doesn't add the query parameter in the request URL (source).
If we have this as pointer, it will prevent adding this query parameter. Otherwise, it will add flush=false every time if user has not set this parameter.

Copy link
Member

Choose a reason for hiding this comment

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

If omitting it is important, then go ahead and use *bool.

@tanyasethi-msft tanyasethi-msft changed the base branch from main to feature/azdatalake/STG83-90 January 17, 2024 03:35
@tanyasethi-msft tanyasethi-msft merged commit a8c6499 into Azure:feature/azdatalake/STG83-90 Jan 17, 2024
35 checks passed
tanyasethi-msft added a commit that referenced this pull request Mar 18, 2024
@tanyasethi-msft
Append With Flush from PR #22245
66a8557
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhendrixMSFT jhendrixMSFT jhendrixMSFT left review comments

@souravgupta-msft souravgupta-msft souravgupta-msft approved these changes

@siminsavani-msft siminsavani-msft Awaiting requested review from siminsavani-msft

@tasherif-msft tasherif-msft Awaiting requested review from tasherif-msft

@gapra-msft gapra-msft Awaiting requested review from gapra-msft gapra-msft is a code owner

@vibhansa-msft vibhansa-msft Awaiting requested review from vibhansa-msft vibhansa-msft is a code owner

@benbp benbp Awaiting requested review from benbp benbp is a code owner

@weshaggard weshaggard Awaiting requested review from weshaggard weshaggard is a code owner

@jsquire jsquire Awaiting requested review from jsquire

@RickWinter RickWinter Awaiting requested review from RickWinter RickWinter is a code owner

@ronniegeraghty ronniegeraghty Awaiting requested review from ronniegeraghty

@ArcturusZhang ArcturusZhang Awaiting requested review from ArcturusZhang ArcturusZhang is a code owner

@lirenhe lirenhe Awaiting requested review from lirenhe lirenhe is a code owner

@tadelesh tadelesh Awaiting requested review from tadelesh tadelesh is a code owner

@richardpark-msft richardpark-msft Awaiting requested review from richardpark-msft richardpark-msft is a code owner

@antkmsft antkmsft Awaiting requested review from antkmsft

@chlowell chlowell Awaiting requested review from chlowell chlowell is a code owner

@heaths heaths Awaiting requested review from heaths heaths is a code owner automatically assigned from Azure/azure-sdk-write-identity

@billwert billwert Awaiting requested review from billwert billwert is a code owner automatically assigned from Azure/azure-sdk-write-identity

@KarishmaGhiya KarishmaGhiya Awaiting requested review from KarishmaGhiya KarishmaGhiya is a code owner automatically assigned from Azure/azure-sdk-write-identity

@schaabs schaabs Awaiting requested review from schaabs schaabs is a code owner automatically assigned from Azure/azure-sdk-write-identity

@mpodwysocki mpodwysocki Awaiting requested review from mpodwysocki mpodwysocki is a code owner automatically assigned from Azure/azure-sdk-write-identity

@xiangyan99 xiangyan99 Awaiting requested review from xiangyan99 xiangyan99 is a code owner automatically assigned from Azure/azure-sdk-write-identity

Assignees
No one assigned
Labels
Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.