Developed 1 new feature, fixed 2 bugs by brightgems · Pull Request #65 · qinxuye/cola

brightgems · 2017-04-24T14:05:01Z

hi chineking,
Issue #64 中讨论的功能(size设置为auto)，功能己实现。另外修复了两个BUG:

MQ中消息存储的中文转码错误
URL模式如果parser返回值，executor中会报错

list(res)

thanks,
brightgems

…ish as soon as all URL/bundle are processed

qinxuye · 2017-04-24T14:29:53Z

cola/core/mq/utils.py

        return obj
    elif isinstance(obj, unicode):
-        return unicode.encode('utf-8')
+        return obj.encode('utf-8','ignore')


嗯，这是个明显的错误。逗号后要加空格。

qinxuye · 2017-04-24T14:30:26Z

cola/core/opener.py

            operation = QNetworkAccessManager.PostOperation
        self.br.load(url, wait_callback=wait_callback, tries=tries, 
-                     operation=operation, body=data, headers=headers)
+                     operation=operation, body=data, headers=headers,load_timeout= self._default_timout)


同样，逗号后加空格。

这些确定的改动（比如格式），可以先提一个commit修正下。

这里空格，再提交改动下哈。

qinxuye · 2017-04-24T14:36:37Z

cola/job/__init__.py


        base = 1 if not self.is_bundle else 1000
        size = self.job_desc.settings.job.size
+        if isinstance(size,basestring):


qinxuye · 2017-04-24T14:44:43Z

cola/job/executor.py

    def __init__(self, *args, **kwargs):
        super(UrlExecutor, self).__init__(*args, **kwargs)
        self.budges = 0
+        self.url_error_times = {}


这里的改动是因为，url是个str类型，而没有error_times属性？

原则上这么改应该没有问题，但为了和bundle做统一，应当在task里送到executor时，判断url类型，如果是str，应当转成Url对象。

url_error_times 在实例化后用到，但没有初始化
···

self.url_error_times[url] = self.url_error_times.get(url, 0) + 1

···

不是，我的意思是，url的error_times，应当是记在Url对象上的，这样再放回mq的时候，也能保存信息，这里放在executor上，等executor退出了，就丢了。

而url有时候在用户parser返回的是个str，我们应当让executor处理的时候转成cola.core.unit.Url对象才是。而error_times的信息记录在Url对象上。bundle也是这么做的。

有道理，这里下一次pr先修改

qinxuye · 2017-04-24T14:45:35Z

cola/job/executor.py

            self._error()
            raise UnitRetryFailed

+


添加行多余了。下同。

qinxuye · 2017-04-24T14:56:54Z

cola/job/executor.py

+                    if self.settings.job.size == 'auto':
+                        inc_budgets = len(next_urls)
+                        if inc_budgets > 0:
+                            self.budget_client.inc_budgets(inc_budgets)


这里，如果next_urls当中存在已经抓取的，这个len(next_urls)就不等于待抓取的urls了吧。

这里确实没有考虑到，似乎需要判断mq里put方法存放的url数量
不过由于现在mq的MessageQueueNodeProxy.put没有返回值；而Store.put其实是返回了保存成功的消息的
是否要先改mq?

这个地方会比较棘手一点，因为MessageQueueNodeProxy在put的时候，并没有真正做放进去的操作，而是对每个其他worker，做了一个cache，当cache满了的时候，才会flush出去。这样做的目的是为了减少网络间的传输开销。

所以，现在put方法并不知道真正放进去了多少个，而去重的操作是在mq的每个节点上才会去做的。

这里budgets的数量大于真实抓取的数量，会导致不能立刻结束么？如果不导致，这里应该就不用改了我理解。

是的，如果budgets的数量大于真实抓取的数量，会导致JOB一直在等待状态。

好像也没有什么好办法，如果遇到有重复的URL，只能按run_loca_job的思路做。如果IDLE超时，自动结束，对吗？

while t.is_alive(): if job.get_status() == FINISHED: break if job.get_status() == IDLE: idle_times += 1 if idle_times > MAX_IDLE_TIMES: break else:

是的，我理解现在只能这样。

qinxuye · 2017-04-24T15:13:25Z

cola/job/task.py

-                                else:
-                                    no_budgets_times = 0
-                                    self._get_unit(curr_priority, self.runnings)
+                                self._get_unit(curr_priority, self.runnings)


这个地方的逻辑改掉了是为什么呢？以前会先申请budgets，现在先取一个unit出来？

如果先申请到budget，然后再取不到unit，实际上什么也没有执行，但budget已经消耗了。这样当size=auto时任务会提前结束，但实际还有URL没有处理完

那我的建议是对是不是auto的情况分开来处理，单独拉一个逻辑，以前的逻辑不动。这样兼容以前的行为。

brightgems · 2017-04-25T12:58:50Z

cola/job/executor.py

                         settings=ReadOnlySettings(self.settings),
                         **options).parse()
-        return list(res)
+        if res:


避免res返回空值后list函数错误

brightgems

多谢回复

brightgems · 2017-04-25T13:32:04Z

cola/job/executor.py

+                    if self.settings.job.size == 'auto':
+                        inc_budgets = len(next_urls)
+                        if inc_budgets > 0:
+                            self.budget_client.inc_budgets(inc_budgets)


这里确实没有考虑到，似乎需要判断mq里put方法存放的url数量
不过由于现在mq的MessageQueueNodeProxy.put没有返回值；而Store.put其实是返回了保存成功的消息的
是否要先改mq?

brightgems · 2017-04-25T13:39:25Z

cola/job/task.py

-                                else:
-                                    no_budgets_times = 0
-                                    self._get_unit(curr_priority, self.runnings)
+                                self._get_unit(curr_priority, self.runnings)


如果先申请到budget，然后再取不到unit，实际上什么也没有执行，但budget已经消耗了。这样当size=auto时任务会提前结束，但实际还有URL没有处理完

brightgems · 2017-04-25T13:40:07Z

cola/core/opener.py

            operation = QNetworkAccessManager.PostOperation
        self.br.load(url, wait_callback=wait_callback, tries=tries, 
-                     operation=operation, body=data, headers=headers)
+                     operation=operation, body=data, headers=headers,load_timeout= self._default_timout)


brightgems · 2017-04-25T13:40:43Z

cola/job/__init__.py


        base = 1 if not self.is_bundle else 1000
        size = self.job_desc.settings.job.size
+        if isinstance(size,basestring):


auto close job if idle for 5 times and auto budget enabled fix lint style

brightgems · 2017-04-26T15:41:06Z

submit code again:

move UrlExecutor.retury_error_times to Url.error_times
auto close job if idle for 5 times and auto budget enabled
fix lint style

qinxuye

我理解现在除了格式问题，应该没有问题了。麻烦再提交commit，然后跑下tests下的所有case，如果都通过，就可以merge了。

qinxuye · 2017-05-01T14:32:41Z

cola/core/opener.py

            operation = QNetworkAccessManager.PostOperation
        self.br.load(url, wait_callback=wait_callback, tries=tries, 
-                     operation=operation, body=data, headers=headers)
+                     operation=operation, body=data, headers=headers,load_timeout= self._default_timout)


这里空格，再提交改动下哈。

qinxuye · 2017-05-01T14:33:03Z

cola/job/container.py

                    all([task.is_idle() for task in self.tasks])
+                if  self.idle_statuses[self.container_id]:
+                    idle_times += 1
+                    if self.job_desc.settings.job.size=='auto' and idle_times > MAX_IDLE_TIMES:


== 两边空格

qinxuye · 2017-05-01T14:34:26Z

cola/job/task.py

-                                        continue
-                                else:
-                                    no_budgets_times = 0
+                                if self.settings.job.size=='auto':


== 两边空格

qinxuye · 2017-05-01T14:34:47Z

cola/job/task.py

+                                if self.settings.job.size=='auto':
                                    self._get_unit(curr_priority, self.runnings)
+                                    # if get unit success, then apply budget, in case budget are wasted
+                                    if len(self.runnings)>0:


两边空格

继盛 and others added 6 commits July 17, 2016 20:52

add cleanup to opener

03d7ba7

Merge branch 'master' into develop

8ec66dd

fix qinxuye#59, require lock before generate a mq store

db3b424

fix encode issue when put msg with chinese

9951b41

enable add_proxy on SpynnerOpener

54c32ac

enable auto increse/decrease budget automatically so that job can fin…

d4f4a3e

…ish as soon as all URL/bundle are processed

qinxuye reviewed Apr 24, 2017

View reviewed changes

qinxuye suggested changes Apr 24, 2017

View reviewed changes

brightgems commented Apr 25, 2017

View reviewed changes

move UrlExecutor.retury_error_times to Url.error_times

f8f314d

auto close job if idle for 5 times and auto budget enabled fix lint style

qinxuye reviewed May 1, 2017

View reviewed changes

Conversation

brightgems commented Apr 24, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qinxuye Apr 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brightgems left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brightgems commented Apr 26, 2017

Uh oh!

qinxuye left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

qinxuye Apr 25, 2017 •

edited

Loading