do not overwrite the stored password with the masked password #1209
Conversation
|
Looks like something is wrong wrt usage of sqlalchemy in your test UPDATE: or not, #1208 has the same error on a different test :| |
|
I'll try to look deeper at this in the next couple of days. Most likely the test I added is doing something wrong. I looked at how other tests were structured and took my best guess. I agree it is strange that #1208 has a similar error. |
| url = 'databaseview/edit/{}'.format(database.id) | ||
| data = {k: database.__getattribute__(k) for k in DatabaseView.add_columns} | ||
| data['sqlalchemy_uri'] = database.safe_sqlalchemy_uri() | ||
| response = self.client.post(url, data=data) |
There was a problem hiding this comment.
I believe you are getting an error here because the database object is modified in the post request and sql alchemy cannot fetch new attributes.
you could try to requery the object after the post request.
| data = {k: database.__getattribute__(k) for k in DatabaseView.add_columns} | ||
| data['sqlalchemy_uri'] = database.safe_sqlalchemy_uri() | ||
| response = self.client.post(url, data=data) | ||
| assert sqlalchemy_uri_decrypted == database.sqlalchemy_uri_decrypted |
There was a problem hiding this comment.
could you please use self.assertEquals instead of assert?
assertEquals gives a nice error message in case of the mismatch
|
Hmm, this is a new build failure after I clicked "update branch". The previous CI build did succeed. I'm not sure if this was from complications from merging 4164d6c or if this is a problem in master brought into this PR by updating the branch. |
|
that looks like transient error |
|
please rebase your branch, otherwise LGTM |
his addresses issue apache#1199
…i does not over-write the stored sqlalchemy_uri
use self.assertEqual for more informative error messages.
dennisobrien
force-pushed
the
dennisobrien/fix_edit_db_mask_password
branch
from
4282718 to
669612f
Compare
|
Rebased and dealt with the merge conflict in tests/core_tests.py |
|
Thanks for following through @dennisobrien ! |
mistercrunch
added
🏷️ bot
This addresses #1199
The fixes a common case of editing a database connection by changing something in the "extra" json but leaving the sqlalchemy uri as-is (and password-masked). Saving this would result in the password being updated with "XXXXXXXXXX". This only updates the password if it is not the password mask.
Added a test as well.