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.

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

noop(cleanup) dedup w/factored-out (dtinit#1392) counter #1393

Merged
merged 3 commits into from
Nov 20, 2024

Conversation

jzacsh
Copy link
Collaborator

@jzacsh jzacsh commented Nov 19, 2024

this utilizes the newly factored out (#1392) streaming counter, so should be noop.

I'm fixing a bug here where the implementation of the microsoft adapter
was reading an arbitrary stream into memory (using DataChunk class), and
instead "just stream directly to the endpoint". It's not _quite_ that
simple though: we actually stream twice, for the very reason that the
old implementation was streaming into memory: we need to know the
filesize.

while we're here I'm doing some "leave the campground better" tasks, so
here's a more nitty-gritty breakdown of this PR:
-  microsoft adapter: marking duplicated code Microsoft{Photo,Video}* as
   needing to rely on newly refactored MicrosoftMedia (so hese kinds of
   bug fixes are easier to maintain, for example).
- DTP: make testability (via DI) easier with for java.net.URL streamers
  (this has already become a pattern, so I just formalized it and
  dropped a TODO in the places that are doing this locally; this way, in
  the future it'll be easier/more obvious what "the pattern" is and how
  to stop maintaining disparate copies of it)
- microsoft adapter: remainder of DataChunk code is now just a POJO, so
  switching to autovalue and letting the test live in the primary user:
  the new `StreamChunker` (as a mini "integrated" test).
- microsoft adapter: all size-related operations are a `long` now
this utilizes the newly factored out streaming counter, so _should_ be
noop.
@jzacsh jzacsh changed the title Codehealth dedupe callableresize noop(cleanup) dedup w/factored-out (dtinit#1390) counter Nov 19, 2024
@jzacsh jzacsh changed the title noop(cleanup) dedup w/factored-out (dtinit#1390) counter noop(cleanup) dedup w/factored-out (dtinit#1392) counter Nov 19, 2024
@jzacsh jzacsh marked this pull request as ready for review November 19, 2024 21:27
@jzacsh
Copy link
Collaborator Author

jzacsh commented Nov 20, 2024

just realized there's already unit tests for this (:portability-transfer:test) so gonna go ahead and merge. will check in with Calum next week when they're back

@jzacsh jzacsh merged commit 4a6b403 into dtinit:master Nov 20, 2024
5 checks passed
@jzacsh jzacsh deleted the codehealth-dedupe-callableresize branch November 20, 2024 20:31
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