modify travis tests to create user with password for mysql and postgres #1092

dennisobrien · 2016-09-12T05:12:19Z

Modify the Travis CI tests to create a database user with a password and use these in the SQLALCHEMY_DATABASE_URI for the tests for mysql and postgres.

This is related to #1070 and I expect this PR to have failing Travis tests.

It's fine to reject this PR -- I just wanted to verify that the tests will fail.

…es and use these in the SQLALCHEMY_DATABASE_URI

xrmx · 2016-09-12T07:16:52Z

@dennisobrien please do experiments on your own fork instead of here

…e 'sqlalchemy_uri' field and the encrypted password in the 'password' field. Modified 'testconn' to work with this. If the passed uri is different from the masked uri, use the passed uri. Otherwise, use the decrypted uri associated with this database.

dennisobrien · 2016-09-15T00:00:51Z

The original intent was to test SQLALCHEMY_DATABASE_URI with a username and password as a test for #1070 (sorry @xrmx -- I only realized after that I could have run the same tests Travis runs on my own machine).

The follow up commits address #1070

At initialization utils.get_or_create_main_db calls dbobj.set_sqlalchemy_uri(config.get("SQLALCHEMY_DATABASE_URI"))
caravel.models.Database.set_sqlalchemy_uri stores the password-masked uri in the field sqlalchemy_uri and the encrypted password in password.
That logic was removed from caravel.views.DatabaseView.pre_add.
- There is still one line in pre_add that I did not move because I don't know where it belongs: utils.merge_perm(sm, 'database_access', db.perm)
This refactor broke testconn so I made some changes here:
- Include database_name in the json payload.
- If uri is the same as database.safe_sqlalchemy_uri(), use the stored database.sqlalchemy_uri_decrypted().
- Otherwise, assume the user is trying a different connection string and test that.
- I really don't know if this is an important feature but it's probably good to respect the user's intent and give accurate feedback.

xrmx · 2016-09-15T07:17:04Z

@dennisobrien If you are going to poke with testconn please cherry-pick whatever seems sensible (the test at least) from this pull request:
#694

Also my logic in views seems a bit easier to follow i think.

dennisobrien · 2016-09-15T18:53:26Z

Thanks @xrmx -- I wasn't aware of that test. I brought in the test but I didn't use git cherry-pick since I needed just part of the commit. I still don't have tests running properly on my dev machine so I'm committing the changes with the hopes that the tests will pass.

Do you recommend that this PR supercedes #694 or that I merge that branch into this one and deal with the merge conflicts?

xrmx · 2016-09-15T19:20:16Z

@dennisobrien no need to do that, i am going to close #694 if all the relevant pieces are included there :)

xrmx · 2016-09-15T19:22:05Z

tests/core_tests.py

+        response = self.client.post('/caravel/testconn', data=data, content_type='application/json')
+        assert response.status_code == 200
+
+        database = (


have you checked this return something with a password?

This test has three parts, but now that I think of it, it should really just use the last two since some of the test cases are against a sqlite db and others use mysql or postgres. So I think I should remove the hard-coded uri here and just rely on the other two tests. One of those two tests with the password masked, the other with the password included in the uri.

This leaves two tests: one that uses the password-masked uri, and the other that uses the uri with the password included.

xrmx · 2016-09-15T19:43:41Z

caravel/views.py

-                .query(models.Database)
-                .filter_by(database_name=database_name)
-                .first()
-            )
            request_uri = request.json.get('uri')


uri = request.json.get('uri')

so that you can save a couple of assigments later?

I did some refactoring here just as you posted this comment. I could skip creating request_uri but I thought I'd be explicit.

I simplified the forking paths here in 8c91119
Thanks for the advice!

dennisobrien · 2016-09-15T20:34:08Z

Hmm, I'm getting a new test failure that has me scratching my head. This passed in the previous Travis run so I'm not sure what's happening here.

======================================================================
ERROR: test_run_async_query (tests.celery_tests.CeleryTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/airbnb/caravel/tests/celery_tests.py", line 235, in test_run_async_query
    df1 = pd.read_sql_query(query1.select_sql, con=eng)
  File "/home/travis/build/airbnb/caravel/.tox/py27-postgres/lib/python2.7/site-packages/pandas/io/sql.py", line 431, in read_sql_query
    parse_dates=parse_dates, chunksize=chunksize)
  File "/home/travis/build/airbnb/caravel/.tox/py27-postgres/lib/python2.7/site-packages/pandas/io/sql.py", line 1190, in read_query
    result = self.execute(*args)
  File "/home/travis/build/airbnb/caravel/.tox/py27-postgres/lib/python2.7/site-packages/pandas/io/sql.py", line 1081, in execute
    return self.connectable.execute(*args, **kwargs)
  File "/home/travis/build/airbnb/caravel/.tox/py27-postgres/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 1991, in execute
    return connection.execute(statement, *multiparams, **params)
  File "/home/travis/build/airbnb/caravel/.tox/py27-postgres/lib/python2.7/site-packages/sqlalchemy/engine/base.py", line 912, in execute
    type(object))
InvalidRequestError: Unexecutable object type: <type 'NoneType'>
-------------------- >> begin captured logging << --------------------
flask_appbuilder.security.sqla.manager: INFO: Updated user admin  user
--------------------- >> end captured logging << ---------------------

dennisobrien · 2016-09-17T00:16:50Z

The remaining questions I have:

Why is DatabaseView.pre_add not called? Is it called for others? Was it called at one point then some dependency changed?
DatabaseView.pre_add is still there but only has a call to utils.merge_perm. Should this be moved to utils.get_or_create_main_db?

xrmx · 2016-09-17T08:51:50Z

Il 17/09/2016 02:16, dennisobrien ha scritto:

The remaining questions I have:

Why is |DatabaseView.pre_add| not called? Is it called for others? Was it
called at one point then some dependency changed?

I've upgraded my test caravel instance to a recent master and it broke
connections to the database, updating my connection uri and saving the database
model fixed the issue. So it looks like the hooks works fine before the password
is clear but not if it's masked?

dennisobrien · 2016-09-19T06:07:03Z

#694 was merged into master. I'm dealing with the merge conflicts in this branch and will push something shortly.

* Single quote filter values with comma * refactor for codeclimate limite * Added unit tests and tooltip

* [sql lab] specify schema name when generating vanila query * Fixing some react warnings

* Add time grain support for time columnd in unix timestamp * Fix datetime parsing for unix epoch Since we've already converted unix epoch to datetime type, we shouldn't specify 'unit' parameter in pandas.to_datetime * Fix SQLite timestamp to datetime conversion

…e#1120) margin_size does not apply. Also nvd3 auto-adjust font-size of axis labels. Temporary solution here: Setting a fixed font-size on nvd3 axis labels and a minimum threshold for label size.

* When the label size is too short, the constant for calculating margin_size does not apply. Also nvd3 auto-adjust font-size of axis labels. Temporary solution here: Setting a fixed font-size on nvd3 axis labels and a minimum threshold for label size. * Only stretch margin for dist_bar

* [explore] giving more room to Slice title * h2->h3 for slice title

* [security] prevent XSS on FAB list views * addressing comments

If ddtm_expr is an expression with special characters then timestamp_grain escapes the special characters already escaped. Solution discussed with sqlalchemy upstream: https://bitbucket.org/zzzeek/sqlalchemy/issues/3737/literal_column-given-a-specific-sql Fix apache#617

Fix apache#596

…e 'sqlalchemy_uri' field and the encrypted password in the 'password' field. Modified 'testconn' to work with this. If the passed uri is different from the masked uri, use the passed uri. Otherwise, use the decrypted uri associated with this database.

dennisobrien · 2016-09-19T06:39:38Z

I've found myself in an endless loop of git rebase in order to try to deal with these branch conflicts but I'm afraid I'm making a bigger mess. If it is simpler to close this PR and have me open another I'm happy to do that.

xrmx · 2016-09-19T08:06:11Z

@dennisobrien do whatever you feel more comfortable with :)

dennisobrien · 2016-09-19T19:18:11Z

Closing this PR and starting a clean PR.

…art (apache#1092)

modify travis tests to create user with password for mysql and postgr…

0feb0ac

…es and use these in the SQLALCHEMY_DATABASE_URI

xrmx mentioned this pull request Sep 14, 2016

problems running caravel with mysql metadata db using mysql username and password #1070

Closed

dennisobrien added 4 commits September 14, 2016 14:57

broke the comment into two lines to make linting happy

a2f2c66

fixed trailing whitespace

1cf298d

fixed a spelling mistake and removed a commented line.

01d1c13

Incorporate the tests from PR apache#694 which addresses apache#596

c795eb9

xrmx reviewed Sep 15, 2016

View reviewed changes

dennisobrien added 2 commits September 15, 2016 12:34

handle the case when a uri is passed but a database name is not

cdb621a

remove the test using the hard-coded sqlalchemy uri.

78588d5

This leaves two tests: one that uses the password-masked uri, and the other that uses the uri with the password included.

xrmx reviewed Sep 15, 2016

View reviewed changes

simplify the logic in testconn. thanks @xrmx

8c91119

mistercrunch and others added 9 commits September 18, 2016 23:11

[sqllab] hide SqlEditor textarea to prevent flicker

6ca1fd9

Single quote filter values with comma (apache#1084)

b9d1abb

* Single quote filter values with comma * refactor for codeclimate limite * Added unit tests and tooltip

[sql lab] specify schema name when generating vanila query (apache#1096)

5fcb6e9

* [sql lab] specify schema name when generating vanila query * Fixing some react warnings

Clean up the druid sync api. (apache#1101)

55145f5

[bugfix] filter_immune_slices doesn't work (apache#1110)

4f64ca0

Animated GIFs on README (apache#1109)

a0c3012

[bugfix] namespacing the mapbox css

aa81482

When the label size is too short, the constant for calculating (apach…

8e661e8

…e#1120) margin_size does not apply. Also nvd3 auto-adjust font-size of axis labels. Temporary solution here: Setting a fixed font-size on nvd3 axis labels and a minimum threshold for label size.

mistercrunch and others added 11 commits September 18, 2016 23:11

[welcome] removing calendar heatmap from welcome page (apache#1119)

190e0d5

[hotfix] lint

b492541

[explore] giving more room to Slice title (apache#1118)

7bfff03

* [explore] giving more room to Slice title * h2->h3 for slice title

[security] prevent XSS on FAB list views (apache#1125)

2542fc8

* [security] prevent XSS on FAB list views * addressing comments

Make sql test connection work with saved Database instance (apache#694)

98da9a0

Fix apache#596

added a newline

f95fb0f

fixing this comment line again

b26a949

dennisobrien closed this Sep 19, 2016

dennisobrien mentioned this pull request Sep 19, 2016

Fix initialization of Database sqlalchemy_uri and password #1137

Merged

dennisobrien deleted the dennisobrien/unitests_db_username_password branch September 19, 2016 19:24

dpgaspar mentioned this pull request Sep 5, 2019

Bump FAB to 2.1.13 #8176

Merged

12 tasks

zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 17, 2021

fix(legacy-plugin-chart-sankey): ensure tooltip position is within ch…

1ad66f1

…art (apache#1092)

zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 24, 2021

fix(legacy-plugin-chart-sankey): ensure tooltip position is within ch…

e301e9d

…art (apache#1092)

zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 25, 2021

fix(legacy-plugin-chart-sankey): ensure tooltip position is within ch…

0b47564

…art (apache#1092)

zhaoyongjie pushed a commit to zhaoyongjie/incubator-superset that referenced this pull request Nov 26, 2021

fix(legacy-plugin-chart-sankey): ensure tooltip position is within ch…

556cd1c

…art (apache#1092)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modify travis tests to create user with password for mysql and postgres #1092

modify travis tests to create user with password for mysql and postgres #1092

dennisobrien commented Sep 12, 2016

xrmx commented Sep 12, 2016

dennisobrien commented Sep 15, 2016

xrmx commented Sep 15, 2016 •

edited

Loading

dennisobrien commented Sep 15, 2016

xrmx commented Sep 15, 2016

xrmx Sep 15, 2016

dennisobrien Sep 15, 2016

xrmx Sep 15, 2016

dennisobrien Sep 15, 2016

dennisobrien Sep 15, 2016

dennisobrien commented Sep 15, 2016

dennisobrien commented Sep 17, 2016

xrmx commented Sep 17, 2016

dennisobrien commented Sep 19, 2016

dennisobrien commented Sep 19, 2016

xrmx commented Sep 19, 2016

dennisobrien commented Sep 19, 2016

modify travis tests to create user with password for mysql and postgres #1092

modify travis tests to create user with password for mysql and postgres #1092

Conversation

dennisobrien commented Sep 12, 2016

xrmx commented Sep 12, 2016

dennisobrien commented Sep 15, 2016

xrmx commented Sep 15, 2016 • edited Loading

dennisobrien commented Sep 15, 2016

xrmx commented Sep 15, 2016

xrmx Sep 15, 2016

Choose a reason for hiding this comment

dennisobrien Sep 15, 2016

Choose a reason for hiding this comment

xrmx Sep 15, 2016

Choose a reason for hiding this comment

dennisobrien Sep 15, 2016

Choose a reason for hiding this comment

dennisobrien Sep 15, 2016

Choose a reason for hiding this comment

dennisobrien commented Sep 15, 2016

dennisobrien commented Sep 17, 2016

xrmx commented Sep 17, 2016

dennisobrien commented Sep 19, 2016

dennisobrien commented Sep 19, 2016

xrmx commented Sep 19, 2016

dennisobrien commented Sep 19, 2016

xrmx commented Sep 15, 2016 •

edited

Loading