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

Port schedule_until feature to upstream #427

Closed
jezdez opened this issue Jun 19, 2018 · 11 comments
Closed

Port schedule_until feature to upstream #427

jezdez opened this issue Jun 19, 2018 · 11 comments
Assignees
Milestone

Comments

@jezdez
Copy link

jezdez commented Jun 19, 2018

Add schedule_until field to queries, to allow expiry (#15, PR: #60)

This is tracked on https://github.com/mozilla/redash/projects/3

@jezdez jezdez added the port upstream whether this needs to be ported upstream label Jun 19, 2018
@alison985 alison985 self-assigned this Jul 1, 2018
@alison985
Copy link

@jezdez I'm having a problem generating the database migration for this. I need to get to a file like this: https://github.com/washort/redash/blob/a2f146ebace7b1025d3d2fa9b7c8c1fd9b5cb408/migrations/versions/eb2f788f997e_.py

If I run these commands:

export FLASK_APP=redash/wsgi.py
flask db migrate

I get this output:

[2018-07-02 02:59:05,922][PID:11016][INFO][alembic.runtime.migration] Context impl PostgresqlImpl.
[2018-07-02 02:59:05,922][PID:11016][INFO][alembic.runtime.migration] Will assume transactional DDL.
Traceback (most recent call last):
  File "/Users/alison/Envs/getredash/bin/flask", line 11, in <module>
    sys.exit(main())
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/flask/cli.py", line 478, in main
    cli.main(args=args, prog_name=name)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/flask/cli.py", line 345, in main
    return AppGroup.main(self, *args, **kwargs)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/click/core.py", line 696, in main
    rv = self.invoke(ctx)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/click/core.py", line 1060, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/click/core.py", line 889, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/flask/cli.py", line 229, in decorator
    return __ctx.invoke(f, *args, **kwargs)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/click/core.py", line 534, in invoke
    return callback(*args, **kwargs)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/flask_migrate/cli.py", line 88, in migrate
    rev_id)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/flask_migrate/__init__.py", line 177, in migrate
    version_path=version_path, rev_id=rev_id)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/command.py", line 176, in revision
    script_directory.run_env()
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/script/base.py", line 427, in run_env
    util.load_python_file(self.dir, 'env.py')
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/util/pyfiles.py", line 81, in load_python_file
    module = load_module_py(module_id, path)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/util/compat.py", line 141, in load_module_py
    mod = imp.load_source(module_id, path, fp)
  File "migrations/env.py", line 87, in <module>
    run_migrations_online()
  File "migrations/env.py", line 80, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/runtime/environment.py", line 836, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/runtime/migration.py", line 321, in run_migrations
    for step in self._migrations_fn(heads, self):
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/command.py", line 156, in retrieve_migrations
    revision_context.run_autogenerate(rev, context)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/autogenerate/api.py", line 415, in run_autogenerate
    self._run_environment(rev, migration_context, True)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/autogenerate/api.py", line 425, in _run_environment
    if set(self.script_directory.get_revisions(rev)) != \
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/script/base.py", line 206, in get_revisions
    return self.revision_map.get_revisions(id_)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py", line 35, in __exit__
    self.gen.throw(type, value, traceback)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/script/base.py", line 174, in _catch_revision_errors
    compat.raise_from_cause(util.CommandError(resolution))
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/util/compat.py", line 205, in raise_from_cause
    reraise(type(exception), exception, tb=exc_tb)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/script/base.py", line 143, in _catch_revision_errors
    yield
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/script/base.py", line 206, in get_revisions
    return self.revision_map.get_revisions(id_)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/script/revision.py", line 299, in get_revisions
    return sum([self.get_revisions(id_elem) for id_elem in id_], ())
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/script/revision.py", line 304, in get_revisions
    for rev_id in resolved_id)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/script/revision.py", line 304, in <genexpr>
    for rev_id in resolved_id)
  File "/Users/alison/Envs/getredash/lib/python2.7/site-packages/alembic/script/revision.py", line 362, in _revision_for_ident
    resolved_id)
alembic.util.exc.CommandError: Can't locate revision identified by '58f810489c47'

The string 58f810489c47 is not in the migrations folder of the upstream repo so I don't know what is causing it to look for that migration. Thoughts?

@alison985
Copy link

Arik's comment: "wait for the updated schedule query interface to be merged"

@rafrombrc rafrombrc added this to the 16 milestone Jul 16, 2018
@rafrombrc rafrombrc modified the milestones: 16, 17 Aug 9, 2018
@jezdez
Copy link
Author

jezdez commented Sep 14, 2018

I think the schedule query interface has landed now upstream.

@jezdez jezdez removed the blocked label Sep 14, 2018
@alison985
Copy link

@jezdez I'm back to the migrations error above.

@jezdez
Copy link
Author

jezdez commented Sep 17, 2018

@alison985 58f810489c47 is a migration added in our fork, 9bcda87#diff-0608868631369d427f4400b7b699dd94 which means that your local environment has had migrations of the fork applied and stored in a migration table that alembic keeps. When it tries to resolve the migration tree it can't find the migration file that the hash refers to (and that it found in the table) and fails to prevent data loss.

The fix is pretty simple, keep an separate clone of the upstream (getredash/redash) repo around and work on patches inside that to keep the migration history separate (by way of running different Docker containers for the upstream and fork repos). E.g. I have a ~/Code/redash-mozilla and a ~/Code/redash-upstream directory and work on them separately. In case you need to port a patch from one to the other you can simply export and import patches using git format-patch and git apply/am.

@emtwo
Copy link

emtwo commented Oct 3, 2018

I wanted to point out: getredash#2426

This does a port of schedule_until as well

@alison985
Copy link

@jezdez Since @emtwo already has a PR open for this I'm changing the assignment of this issue to her and removing from my task board.

@alison985 alison985 assigned emtwo and unassigned alison985 Oct 4, 2018
@jezdez
Copy link
Author

jezdez commented Oct 4, 2018

@alison985 Understood.

@emtwo Since the feature already exists in our fork we'll need to write a data migration to port over the "schedule_until" values to the new JSON schedule field and delete the column afterwards. Can you think of huge blockers for that problem? Would you mind preparing such a migration for when we pull the changes with the merged PR into the fork?

@emtwo
Copy link

emtwo commented Oct 4, 2018

@jezdez I don't think there should be any blockers to this. Yes, I will create this migration.

@rafrombrc rafrombrc modified the milestones: 17, 18, 19 Nov 7, 2018
@rafrombrc rafrombrc modified the milestones: 19, 20 Nov 14, 2018
@rafrombrc
Copy link
Member

Once #187 (which contains an implementation of this functionality) is merged upstream and then brought into our codebase, we'll need to write a migration to move our deployment's 'schedule_until' data into the new format.

@jezdez
Copy link
Author

jezdez commented Apr 3, 2019

Done and landed in prod.

@jezdez jezdez closed this as completed Apr 3, 2019
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

4 participants