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

unfurling: integrate Deliverer with the unfurl machinery CORE-9248 #14611

Merged
merged 27 commits into from Nov 12, 2018

Conversation

Projects
None yet
2 participants
@mmaxim
Member

mmaxim commented Nov 7, 2018

Patch does the following:

1.) Introduce new class Unfurler, whose main job is to run unfurling end-to-end, and to queue up a chat message with the unfurl. Maintain the state of all unfurls, and allow outsiders to know about that state for the purposes of sending the unfurl message.
2.) Refactor ConversationBackedStorage to use a uid as a parameter instead of pulling from global state.
3.) Integrate Deliverer with Unfurler in a similar manner as attachments.Uploader. That is, we check to make sure unfurling is complete before sending the message. We do not count unfurling in progress as a failure, and it also doesn't block any other messages.

@mmaxim mmaxim requested a review from joshblum Nov 7, 2018

mmaxim added some commits Nov 7, 2018

Show resolved Hide resolved go/chat/sender.go Outdated
Show resolved Hide resolved go/chat/sender.go Outdated
Show resolved Hide resolved go/chat/unfurl/scraper_test.go Outdated
Show resolved Hide resolved go/chat/unfurl/unfurler.go Outdated
Show resolved Hide resolved go/chat/unfurl/unfurler_test.go
Show resolved Hide resolved go/chat/unfurl/unfurler.go Outdated
Show resolved Hide resolved go/chat/unfurl/unfurler.go
@joshblum

This comment has been minimized.

Member

joshblum commented Nov 10, 2018

mmaxim added some commits Nov 11, 2018

@mmaxim mmaxim merged commit f378de9 into master Nov 12, 2018

1 of 2 checks passed

ci/circleci Your tests failed on CircleCI
Details
continuous-integration/jenkins/pr-head This commit looks good
Details

@mmaxim mmaxim deleted the mike/CORE-9248 branch Nov 12, 2018

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