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

Query schedule fails if there "Until" field equals None #3341

Closed
denisov-vlad opened this issue Jan 25, 2019 · 8 comments
Closed

Query schedule fails if there "Until" field equals None #3341

denisov-vlad opened this issue Jan 25, 2019 · 8 comments

Comments

@denisov-vlad
Copy link
Member

denisov-vlad commented Jan 25, 2019

Issue Summary

Query schedule doesn't work if there "until" field equals None:

[2019-01-25 10:36:45,288][PID:12495][ERROR][ForkPoolWorker-3448] Task redash.tasks.refresh_queries[dc3a6d03-fc3e-4e14-8fb9-d5de71ac2b59] raised unexpected: TypeError("'NoneType' object has no attribute '__getitem__'",)
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/dist-packages/celery/app/trace.py", line 382, in trace_task
    R = retval = fun(*args, **kwargs)
  File "/opt/redash/redash.origin.latest/redash/worker.py", line 75, in __call__
    return TaskBase.__call__(self, *args, **kwargs)
  File "/usr/local/lib/python2.7/dist-packages/celery/app/trace.py", line 641, in __protected_call__
    return self.run(*args, **kwargs)
  File "/opt/redash/redash.origin.latest/redash/tasks/queries.py", line 286, in refresh_queries
    for query in models.Query.outdated_queries():
  File "/opt/redash/redash.origin.latest/redash/models/__init__.py", line 564, in outdated_queries
    query.schedule['until'], '%Y-%m-%d')) if query.schedule['until'] else None
TypeError: 'NoneType' object has no attribute '__getitem__'

Schedule from queries table: {"interval": 300, "until": null, "day_of_week": null, "time": null}

It can't parse null value.

Is it ok to remove "until" field if it equals null ?

Technical details:

  • Redash Version: master
  • Browser/OS: –
  • How did you install Redash: –
@denisov-vlad denisov-vlad changed the title Query schedule fails if there is not "Until" field Query schedule fails if there "Until" field equals None Jan 25, 2019
@ranbena
Copy link
Contributor

ranbena commented Jan 30, 2019

@arikfr How bow compacting before saving? Would be more space efficient as well?

Before:

save() {
  ...
  if (newSchedule.interval) {
    this.props.updateQuery({ schedule: clone(newSchedule) });
  } else {
    this.props.updateQuery({ schedule: null });
  }
  ...
}

After:

import { chain, isNil } from 'lodash';

save() {
  ...
  const schedule = chain(newSchedule).clone().omitBy(isNil);
  this.props.updateQuery({ schedule });
  ...
}

@arikfr
Copy link
Member

arikfr commented Jan 31, 2019

@ranbena it sounds like a good idea to compact the values. But will the new version you suggested result in schedule = null when it's empty?

@arikfr arikfr self-assigned this Jan 31, 2019
@ranbena
Copy link
Contributor

ranbena commented Jan 31, 2019

will the new version you suggested result in schedule = null when it's empty?

Bahhh.. I'll submit a tested front end PR.

@ranbena ranbena self-assigned this Jan 31, 2019
@arikfr
Copy link
Member

arikfr commented Jan 31, 2019

@denisov-vlad I'm looking at the exception you got and I think it happens because schedule itself is None.

If you run this code:

>>> import pytz
>>> schedule = {'until': None}
>>> schedule_until = pytz.utc.localize(datetime.datetime.strptime(schedule['until'], '%Y-%m-%d')) if schedule['until'] else None
>>> schedule_until is None
True

You can see that it works as expected. But if you try the following --

>>> schedule = None
>>> schedule_until = pytz.utc.localize(datetime.datetime.strptime(schedule['until'], '%Y-%m-%d')) if schedule['until'] else None
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'NoneType' object has no attribute '__getitem__'

It generates the error you got.

The strange part is that we load only queries with schedule is not null:

.filter(Query.schedule.isnot(None))

So I'm not sure how you got this error 🤔

@arikfr
Copy link
Member

arikfr commented Jan 31, 2019

To make the logic there easier to follow, I rewrote it a bit in 91ba67c.

@arikfr
Copy link
Member

arikfr commented Feb 4, 2019

@denisov-vlad did you manage to reproduce this or understand why it happens?

@denisov-vlad
Copy link
Member Author

I've found that empty schedule had two types of null values: real NULL and 'null' as string. I've replaced 'null' to NULL and now I don't see this errors.

@denisov-vlad
Copy link
Member Author

Also I've checked new queries after 'null'->NULL update and didn't see 'null'.

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

No branches or pull requests

3 participants