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

Updated Daybook posts importer to log bytes #1198

Merged
merged 2 commits into from Dec 14, 2022

Conversation

saratchebrolu
Copy link
Contributor

@saratchebrolu saratchebrolu commented Dec 12, 2022

Updated Daybook posts importer to log bytes

NOTE: Dependent on #1197

Unlike in the case of other services, we don't seem to upload images in the importer. Instead, image URLs are being included in the form data, so, the size of the same is being logged.

Summary: 

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D41685921
@saratchebrolu saratchebrolu marked this pull request as ready for review December 12, 2022 15:56
Copy link
Collaborator

@alexeyqu-fb alexeyqu-fb left a comment

Choose a reason for hiding this comment

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

LGTM, provided we want to log the formBody size here.

@@ -155,6 +164,7 @@ private String insertActivity(SocialActivityModel activity, TokensAndUrlAuthData

FormBody formBody = builder.build();
requestBuilder.post(formBody);
size = formBody.contentLength();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok but are we sure we want to log the payload size here?
E.g. what if the Daybook unpacks it / some fields in the form are stored more efficiently?

I'd say let's merge this and then check the numbers for real transfers..

@alexeyqu-fb alexeyqu-fb merged commit bde428a into google:master Dec 14, 2022
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.

None yet

2 participants