X Tutup
Skip to content

internxt: implement OpenChunkWriter support#9233

Open
jzunigax2 wants to merge 1 commit intorclone:masterfrom
internxt:master
Open

internxt: implement OpenChunkWriter support#9233
jzunigax2 wants to merge 1 commit intorclone:masterfrom
internxt:master

Conversation

@jzunigax2
Copy link
Contributor

@jzunigax2 jzunigax2 commented Mar 5, 2026

What is the purpose of this change?

Implement OpenChunkWriter on the internxt backend to allow for better memory management during file operations.

Was the change discussed in an issue or in the forum before?

#9162

Checklist

  • I have read the contribution guidelines.
  • I have added tests for all changes in this PR if appropriate.
  • I have added documentation for the changes if appropriate.
  • All commit messages are in house style.
  • I'm done, this Pull Request is ready for review :-)

@jzunigax2
Copy link
Contributor Author

Tests passing:
imagen

  • note I had to exclude OpenChunkWriter from features as we restrict multi part uploads for files smaller than 100MBs, some tests were failling due to this restriction

Copy link
Member

@ncw ncw left a comment

Choose a reason for hiding this comment

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

I gave this a quick once over - looks great :-)

Can you fix the lint error?

What do you mean by

note I had to exclude OpenChunkWriter from features as we restrict multi part uploads for files smaller than 100MBs, some tests were failling due to this restriction

You've got this in your code which should be all you need. Where are the tests excluded - I don't see that?

                ChunkedUpload: fstests.ChunkedUploadConfig{
			MinChunkSize:       100 * fs.Mebi,
			NeedMultipleChunks: true,
		},

@jzunigax2
Copy link
Contributor Author

You've got this in your code which should be all you need. Where are the tests excluded - I don't see that?

                ChunkedUpload: fstests.ChunkedUploadConfig{
			MinChunkSize:       100 * fs.Mebi,
			NeedMultipleChunks: true,
		},

I had two failling tests TestMultithreadCopy/upload=true,size=62914561,streams=2 and TestIntegration/FsMkdir/FsOpenChunkWriter. One where it tests a streamed file which we reject due to unknown size and the other which we reject due to it's small size. The only way I found to skip these tests was by setting f.features.OpenChunkWriter = nil manually right after .fill() see here

@jzunigax2 jzunigax2 requested a review from ncw March 5, 2026 20:40
@ncw
Copy link
Member

ncw commented Mar 6, 2026

The only way I found to skip these tests was by setting f.features.OpenChunkWriter = nil manually right after .fill() see here

Ah, if you do that then you disable the feature and this pr is pointless!

It sounds like the first failure is a bug in the tests.

The second can be fixed by Skip the test if features.PutStream ==0

Do you want to try to fix the integration tests?

@jzunigax2
Copy link
Contributor Author

Ah, if you do that then you disable the feature and this pr is pointless!

Oh, then definetely let me remove that line and ensure tests pass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

X Tutup