redash
                                
                                 redash copied to clipboard
                                
                                    redash copied to clipboard
                            
                            
                            
                        Sqlalchemy 2.0
What type of PR is this?
Upgraded SQLAlchemy 1.3 -> 2.0
- [ ] Refactor
- [ ] Feature
- [ ] Bug Fix
- [ ] New Query Runner (Data Source)
- [ ] New Alert Destination
- [ ] Other
How is this tested?
- [x] Unit tests (pytest, jest)
- [x] E2E Tests (Cypress)
- [x] Manually
- [ ] N/A
@guidopetri @konnectr If it is okay to merge this from your perspective, please let me know. If so, I will consider this again.
@masayuki038 besides:
- removing -p option in ci
- moving Alerts states to a separate Enum
other changes were required:
- paginator changes were needed cause paginator function from flask-sqlachemy has changed
- removing relative imports was related to issues in sqlalchemy
- sort_query function was completely removed from sqlalchemy-utils, so it had to be replaced
- all json changes were related to a bunch of issues in tests with a filtering in whereclause
sqlalchemy dependencies introduce lots of changes, which do not allow to prepare tiny diffs. god bless flask and all its dependencies was not required to be upgraded together with sqlalchemy deps
@masayuki038 could you please trigger ci from the latest commit, I've fixed lint there
It looks ci is running now... https://github.com/getredash/redash/actions/runs/7298330872
It looks ci is running now... https://github.com/getredash/redash/actions/runs/7298330872
build for a latest commit was not triggered
@AndrewChubatiuk I manually re-run Github Action, but I may not have been able to fetch the latest code. I tried deleting my review, but the status did not change.
Could you try pushing something? Or could you please close this and create another PR?
@masayuki038 please approve this pipeline https://github.com/getredash/redash/actions/runs/7307066364
@masayuki038 some changes were lost during rebase. pushed once again
Codecov Report
Attention: Patch coverage is 87.09677% with 56 lines in your changes are missing coverage. Please review.
Project coverage is 63.90%. Comparing base (
b7f22b1) to head (05bbe48). Report is 1 commits behind head on master.
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6671      +/-   ##
==========================================
+ Coverage   63.80%   63.90%   +0.09%     
==========================================
  Files         161      162       +1     
  Lines       13087    13175      +88     
  Branches     1812     1824      +12     
==========================================
+ Hits         8350     8419      +69     
- Misses       4434     4446      +12     
- Partials      303      310       +7     
| Files | Coverage Δ | |
|---|---|---|
| redash/alerts.py | 100.00% <100.00%> (ø) | |
| redash/app.py | 100.00% <100.00%> (ø) | |
| redash/authentication/__init__.py | 81.44% <100.00%> (+0.09%) | :arrow_up: | 
| redash/cli/organization.py | 91.66% <100.00%> (+0.23%) | :arrow_up: | 
| redash/destinations/asana.py | 81.48% <100.00%> (ø) | |
| redash/handlers/alerts.py | 86.74% <100.00%> (+0.16%) | :arrow_up: | 
| redash/handlers/data_sources.py | 72.05% <100.00%> (ø) | |
| redash/handlers/destinations.py | 77.92% <100.00%> (ø) | |
| redash/handlers/embed.py | 87.50% <100.00%> (ø) | |
| redash/handlers/organization.py | 81.81% <100.00%> (+1.81%) | :arrow_up: | 
| ... and 41 more | 
I understand correctly that we are still abandoning simplejson ?
I will try to test the operation in manual mode in the near future. It is unlikely that our auto-tests cover all possible problems
@konnectr Thanks! I will start to review this next week as well.
@masayuki038 @justinclift is anyone able to review it? I've done all requested changes and answered all questions
@AndrewChubatiuk Thanks for your updating. I will check it.
@AndrewChubatiuk I checked and commented. Could you check them?
thanks @masayuki038, made updates, please check again
@AndrewChubatiuk I checked and marked "Resolved" in some of them. And I left some comments. Could you check them?
@masayuki038 already checked them. answered or made code changes
@AndrewChubatiuk Could you stop to update the things other than "Sqlalchemy 2.0"?
@AndrewChubatiuk Could you stop to update the things other than "Sqlalchemy 2.0"?
@masayuki038 i update only things, which are impacted by pr comments changes
@AndrewChubatiuk OK. But, I don't think we need to update "assert_called_once_with("requests.redash_ping.get", ANY)" to "assert_called" like the above thread by upgrading to SQLAlchemy 2.0... Why did you update it yesterday?
@AndrewChubatiuk OK. But, I don't think we need to update "assert_called_once_with("requests.redash_ping.get", ANY)" to "assert_called" like the above thread by upgrading to SQLAlchemy 2.0... Why did you update it yesterday?
replied in a thread