Publish to telegram #15

sgaynetdinov · 2019-11-16T10:35:00Z

old PR #14

close #13

sgaynetdinov · 2019-11-16T15:17:28Z

app/main.go

+	tg, err := proc.NewTelegramClient(opts.TG)
+	if err != nil {
+		log.Fatalf("[ERROR] failed initilization telegram client %s, %v", opts.TG, err)
+	}


when starting without TELEGRAM_TOKEN there will be an error,

is this the right behavior?

need to register this env in docker-compose?

this is not right, telegram support should be optional

for "need to register this env in docker-compose?" - I think you are asking if it should be passed via docker-compose? Yes, if a user needs to set (or pass) TELEGRAM_TOKEN it has to be defined/declared in the compose

app/proc/telegram_test.go

umputun · 2019-11-18T17:54:54Z

Looks good to me, I think I can merge it as-is, but first need to test it locally.

In the meantime, I would suggest 2 more changes (we can add it after the merge, up to you):

Declare and use some Notifyer interface instead of direct TelegramClient in Processor. This will allow removing direct dependency and will help testing Processor without the actual telegram client involved.
I'm not sure how timeouts handled here. None of telegram related methods seem to set timeout and none of them seems to use context for termination. We shouldn't hang indefinitely on failed connection to telegram but continue the normal operation after printing WARN level log message.

umputun · 2019-11-18T21:49:52Z

I can't make it work. Created a new token, opened a new channel and all I get is "api error: Bad Request: chat not found". Btw, the message is not too helpful, knowing what chat is not found will be nice. In addition, it doesn't respect the lack of telegram_channel defined and trying to send on any feed.

It is also unclear to me if you expect me to set a channel with @ prefix or this library dealing with it.

umputun · 2019-11-19T18:04:30Z

Ok, this one is running fine, I got a bunch of messages which is good. They also seem to be formatted nice, which is also good. However, none of them publishes mp3, they all links regardless of the size for mp3. From your earlier screenshots, I thought this is going to have media published if size allows (under 50m) but looks like it doesn't do it at all.

sgaynetdinov · 2019-11-20T14:03:47Z

I'm not sure how timeouts handled here. None of telegram related methods seem to set timeout and none of them seems to use context for termination. We shouldn't hang indefinitely on failed connection to telegram but continue the normal operation after printing WARN level log message.

Add timeouts 69f08f9

sgaynetdinov · 2019-11-20T14:18:24Z

Ok, this one is running fine, I got a bunch of messages which is good. They also seem to be formatted nice, which is also good. However, none of them publishes mp3, they all links regardless of the size for mp3. From your earlier screenshots, I thought this is going to have media published if size allows (under 50m) but looks like it doesn't do it at all.

Early, I published in preview mode only one link and telegram published audio itself.
When the message consists of many links, not working as expected, shown the first link in a message (not audio).
Now I check the file size and attach it to the message (in process).

umputun · 2019-11-20T21:54:29Z

app/proc/telegram.go

+	response := make(chan *responseTelegram)
+	go client.send(channelID, item, response)
+
+	select {


it looks wrong as it leaks goroutine on timeout. Timeout support has to be or on telegram's client level or on the level of http client injected to the telegram client.

see https://github.com/tucnak/telebot/blob/v2/bot.go#L69 - you can pass http client and set timeout for the client, without any need in this dangerous workaround with goroutine.

use http.Client 69f08f9

This reverts commit 69f08f9.

sgaynetdinov · 2019-11-27T15:41:33Z

In process of researching, I didn't find how to send in one message including text and audio. In documentation of Telegram I found Sending Messages in Bulk, but I don't know how to use it in package telebot.
Bot can send at first text and after audio, but then messages shuffle (if messages more than two)

It is possible to achieve result as on the screenshot only if this file will be not big.

umputun · 2019-11-28T03:50:40Z

I'd suggest 3 for files under 50M and just a text (with a link to mp3) for bigger files

sgaynetdinov · 2019-12-04T20:54:33Z

sgaynetdinov · 2019-12-04T20:55:36Z

app/proc/telegram.go

+	return err
+}
+
+func getContentLength(url string) (int64, error) {


@umputun how mocking http response?

see httptest. You can run test server and provide whatever response you need

this is an example of httptest.Server use in real life, from one of my projects https://github.com/go-pkgz/rest/blob/master/httperrors_test.go#L16

sgaynetdinov · 2019-12-04T20:56:48Z

app/proc/telegram.go

+}
+
+func (client TelegramClient) downloadAudio(url string) (*[]byte, error) {
+	clientHTTP := &http.Client{Timeout: 60 * 10 * time.Second}


@umputun I set big timeout, it is OK ?

hard-coded timeout is something to avoid. But I can add it to opts and pass in latter on

umputun · 2019-12-04T22:36:28Z

app/proc/telegram.go

+	return message, err
+}
+
+func (client TelegramClient) downloadAudio(url string) (*[]byte, error) {


no need in pointer return here, []byte is a slice backed by pointer already

umputun · 2019-12-04T22:37:26Z

app/proc/telegram.go

-	return err
+	audio := tb.Audio{
+		File:     tb.FromReader(bytes.NewReader(*file)),
+		FileName: "1.mp3",


1.mp3 ? probably should be the file name from the feed?

umputun · 2019-12-04T22:38:24Z

app/proc/telegram.go

+	defer resp.Body.Close()
+
+	if resp.StatusCode != 200 {
+		return 0, errors.New("status code != 200")


probably include the code in error will be useful

umputun · 2019-12-04T22:39:35Z

app/proc/telegram.go

+
+	var message *tb.Message
+
+	if contentLength < 50_000_000 {


moving 50_000_000 to a const and naming it logically, like maxTelegramFileSize or smth like this will be nice

app/proc/telegram_test.go

umputun · 2019-12-05T07:08:53Z

app/proc/telegram.go

+
+	log.Printf("[DEBUG] start download audio: %s", url)
+
+	file, err := ioutil.ReadAll(resp.Body)


This code reads file content to memory. For 50M file it is not too bad, but running multiple feeds at the same time can make it way too big. I don't even see why you need to read it to memory at all. Looks like all you need in your consumer is io.Reader, i.e. resp.Body can be passed to tb.FromReader directly

umputun · 2019-12-06T02:12:31Z

I still see a few minor things, but generally, it seems to be ok and working. I'll merge it and we could address things after that.

umputun · 2019-12-06T02:50:11Z

@sgaynetdinov thx for your work on PR. I have it merged and fixed a bunch of small things after the merge.

sgaynetdinov · 2019-12-06T11:46:10Z

@umputun thanks for your review on my code

app/proc/telegram.go

sgaynetdinov added 3 commits November 16, 2019 15:12

Add telegram client, for send notification

d938e0f

Send message telegram if get new item RSS

908cda8

Add new package in vendor

3ae8267

sgaynetdinov marked this pull request as ready for review November 16, 2019 15:04

sgaynetdinov commented Nov 16, 2019

View reviewed changes

sgaynetdinov added 7 commits November 18, 2019 18:09

Not raise error if telegram token empty

d8fb331

Add 'TestSendIfChannelIDEmpty'

c741b89

Fix: 'NewTelegramClient' return 'TelegramClient' not 'nil'

ee439da

Add in environment 'TELEGRAM_TOKEN'

635a674

Refactoring test

80ca198

Add test for 'TelegramClient.tagLinkOnlySupport'

be4eeb7

Add test for 'TelegramClient.getMessageHTML'

117ef40

golangcibot reviewed Nov 18, 2019

View reviewed changes

app/proc/telegram_test.go Outdated Show resolved Hide resolved

Rename 'html_expected' -> 'htmlExpected'

bec8544

sgaynetdinov requested a review from umputun November 18, 2019 16:08

sgaynetdinov added 3 commits November 19, 2019 12:01

Pretty log, if error send telegram message

010a331

dd prefix "@" for "channel" if not found

d3d0adb

Add interface 'Notification'

b8312f0

Set timeout for telegram

69f08f9

umputun reviewed Nov 20, 2019

View reviewed changes

sgaynetdinov added 2 commits November 26, 2019 16:34

Revert "Set timeout for telegram"

2b98130

This reverts commit 69f08f9.

Use timeout from http.Client without context timeout

336fa15

umputun mentioned this pull request Dec 1, 2019

Publish to telegram using python #17

Closed

sgaynetdinov added 2 commits December 4, 2019 17:59

Pretty log

c5a94a5

Send audio if file size less 50 Mb or send only text

c2f5de1

sgaynetdinov commented Dec 4, 2019

View reviewed changes

umputun reviewed Dec 4, 2019

View reviewed changes

sgaynetdinov added 3 commits December 5, 2019 11:12

Add const 'maxTelegramFileSize'

727df45

Refactoring, message error

395831a

Get filename audio file from url

ec63f0e

golangcibot reviewed Dec 5, 2019

View reviewed changes

app/proc/telegram_test.go Show resolved Hide resolved

app/proc/telegram_test.go Show resolved Hide resolved

Fix: []byte is a slice backed by pointer already

588a63f

umputun reviewed Dec 5, 2019

View reviewed changes

sgaynetdinov added 2 commits December 5, 2019 22:25

Telegram timeout moved to opts

e27a045

Not reads file content to memory

7193518

umputun merged commit 5121af2 into umputun:master Dec 6, 2019

umputun added a commit that referenced this pull request Dec 6, 2019

a lot of small fixes post #15

22e7cf9

sgaynetdinov mentioned this pull request Dec 6, 2019

Add test 'telegram.getContentLength' #19

Merged

umputun mentioned this pull request Jan 1, 2020

Allow >50M audio upload to telegram #23

Closed

paskal reviewed Feb 15, 2020

View reviewed changes

app/proc/telegram.go Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Publish to telegram #15

Publish to telegram #15

sgaynetdinov commented Nov 16, 2019 •

edited

Loading

sgaynetdinov Nov 16, 2019

umputun Nov 16, 2019

umputun Nov 16, 2019 •

edited

Loading

sgaynetdinov Nov 18, 2019

umputun commented Nov 18, 2019

umputun commented Nov 18, 2019

umputun commented Nov 19, 2019

sgaynetdinov commented Nov 20, 2019

sgaynetdinov commented Nov 20, 2019 •

edited

Loading

umputun Nov 20, 2019

umputun Nov 20, 2019 •

edited

Loading

sgaynetdinov Nov 26, 2019

sgaynetdinov commented Nov 27, 2019

umputun commented Nov 28, 2019

sgaynetdinov commented Dec 4, 2019

sgaynetdinov Dec 4, 2019

umputun Dec 5, 2019

umputun Dec 5, 2019

sgaynetdinov Dec 4, 2019 •

edited

Loading

umputun Dec 4, 2019

umputun Dec 4, 2019

umputun Dec 4, 2019

umputun Dec 4, 2019

umputun Dec 4, 2019

umputun Dec 5, 2019

umputun commented Dec 6, 2019

umputun commented Dec 6, 2019

sgaynetdinov commented Dec 6, 2019


		log.Printf("[DEBUG] start download audio: %s", url)

		file, err := ioutil.ReadAll(resp.Body)

Publish to telegram #15

Publish to telegram #15

Conversation

sgaynetdinov commented Nov 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

umputun Nov 16, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

umputun commented Nov 18, 2019

umputun commented Nov 18, 2019

umputun commented Nov 19, 2019

sgaynetdinov commented Nov 20, 2019

sgaynetdinov commented Nov 20, 2019 • edited Loading

Choose a reason for hiding this comment

umputun Nov 20, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgaynetdinov commented Nov 27, 2019

umputun commented Nov 28, 2019

sgaynetdinov commented Dec 4, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sgaynetdinov Dec 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

umputun commented Dec 6, 2019

umputun commented Dec 6, 2019

sgaynetdinov commented Dec 6, 2019

sgaynetdinov commented Nov 16, 2019 •

edited

Loading

umputun Nov 16, 2019 •

edited

Loading

sgaynetdinov commented Nov 20, 2019 •

edited

Loading

umputun Nov 20, 2019 •

edited

Loading

sgaynetdinov Dec 4, 2019 •

edited

Loading