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

[logs] Dropping dt column #4587

Merged
merged 1 commit into from
Apr 11, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions UPDATING.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ assists people when migrating to a new version.
## Superset 0.23.0

* [4565](https://github.com/apache/incubator-superset/pull/4565)
* [4587](https://github.com/apache/incubator-superset/pull/4587)
26 changes: 26 additions & 0 deletions superset/migrations/versions/30bb17c0dc76_.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
"""empty message

Revision ID: 30bb17c0dc76
Revises: f231d82b9b26
Create Date: 2018-04-08 07:34:12.149910

"""

# revision identifiers, used by Alembic.
revision = '30bb17c0dc76'
down_revision = 'f231d82b9b26'

from datetime import date

from alembic import op
import sqlalchemy as sa


def upgrade():
with op.batch_alter_table('logs') as batch_op:
batch_op.drop_column('dt')
Copy link
Member

Choose a reason for hiding this comment

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

@john-bodley let's avoid backward incompatible db migrations in the future as they require scheduling downtime to upgrade the app for people who use rolling deploys. This took our production down for a moment.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mistercrunch just to clarify what should have the logic been here? I'm simply asking to learn.

Copy link
Member

Choose a reason for hiding this comment

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

Not sure, maybe we could have just left the column there as NULL moving forward and add a note about it being deprecated.



def downgrade():
with op.batch_alter_table('logs') as batch_op:
batch_op.add_column(sa.Column('dt', sa.Date, default=date.today()))
5 changes: 2 additions & 3 deletions superset/models/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from __future__ import unicode_literals

from copy import copy, deepcopy
from datetime import date, datetime
from datetime import datetime
import functools
import json
import logging
Expand All @@ -20,7 +20,7 @@
import pandas as pd
import sqlalchemy as sqla
from sqlalchemy import (
Boolean, Column, create_engine, Date, DateTime, ForeignKey, Integer,
Boolean, Column, create_engine, DateTime, ForeignKey, Integer,
MetaData, select, String, Table, Text,
)
from sqlalchemy.engine import url
Expand Down Expand Up @@ -865,7 +865,6 @@ class Log(Model):
user = relationship(
security_manager.user_model, backref='logs', foreign_keys=[user_id])
dttm = Column(DateTime, default=datetime.utcnow)
dt = Column(Date, default=date.today())
duration_ms = Column(Integer)
referrer = Column(String(1024))

Expand Down
18 changes: 0 additions & 18 deletions superset/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1472,24 +1472,6 @@ def checkbox(self, model_view, id_, attr, value):
db.session.commit()
return json_success('OK')

@api
@has_access_api
@expose('/activity_per_day')
def activity_per_day(self):
"""endpoint to power the calendar heatmap on the welcome page"""
Log = models.Log # noqa
qry = (
db.session
.query(
Log.dt,
sqla.func.count())
.group_by(Log.dt)
.all()
)
payload = {str(time.mktime(dt.timetuple())):
ccount for dt, ccount in qry if dt}
return json_success(json.dumps(payload))

@api
@has_access_api
@expose('/schemas/<db_id>/')
Expand Down
1 change: 0 additions & 1 deletion tests/base_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ def assert_can_all(view_menu):

self.assertIn(('can_add_slices', 'Superset'), gamma_perm_set)
self.assertIn(('can_copy_dash', 'Superset'), gamma_perm_set)
self.assertIn(('can_activity_per_day', 'Superset'), gamma_perm_set)
self.assertIn(('can_created_dashboards', 'Superset'), gamma_perm_set)
self.assertIn(('can_created_slices', 'Superset'), gamma_perm_set)
self.assertIn(('can_csv', 'Superset'), gamma_perm_set)
Expand Down
1 change: 0 additions & 1 deletion tests/security_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ def assert_can_gamma(self, perm_set):

self.assertIn(('can_add_slices', 'Superset'), perm_set)
self.assertIn(('can_copy_dash', 'Superset'), perm_set)
self.assertIn(('can_activity_per_day', 'Superset'), perm_set)
self.assertIn(('can_created_dashboards', 'Superset'), perm_set)
self.assertIn(('can_created_slices', 'Superset'), perm_set)
self.assertIn(('can_csv', 'Superset'), perm_set)
Expand Down