Skip to content

[SYSADMIN ACTION][Dependency] Bump sqlalchemy from 1.4.29 to 2.0.41 in /.setup/pip #11578

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

Merged
merged 51 commits into from
Jul 25, 2025

Conversation

dependabot[bot]
Copy link
Contributor

@dependabot dependabot bot commented on behalf of github Apr 1, 2025

Please see the release notes:
https://submitty.org/sysadmin/installation/version_notes/v25.07.01

Bumps sqlalchemy from 1.4.29 to 2.0.40.
In order to test this PR, you would need to run submitty_install_bin as the dependencies are not updated when you run submitty_install
Related to Submitty/submitty.github.io#692

Release notes

Sourced from sqlalchemy's releases.

2.0.40

Released: March 27, 2025

orm

  • [orm] [bug] Fixed regression which occurred as of 2.0.37 where the checked ArgumentError that's raised when an inappropriate type or object is used inside of a Mapped annotation would raise TypeError with "boolean value of this clause is not defined" if the object resolved into a SQL expression in a boolean context, for programs where future annotations mode was not enabled. This case is now handled explicitly and a new error message has also been tailored for this case. In addition, as there are at least half a dozen distinct error scenarios for intepretation of the Mapped construct, these scenarios have all been unified under a new subclass of ArgumentError called MappedAnnotationError, to provide some continuity between these different scenarios, even though specific messaging remains distinct.

    References: #12329

  • [orm] [bug] Fixed regression in ORM Annotated Declarative class interpretation caused by typing_extension==4.13.0 that introduced a different implementation for TypeAliasType while SQLAlchemy assumed that it would be equivalent to the typing version, leading to pep-695 type annotations not resolving to SQL types as expected.

    References: #12473

sql

  • [sql] [usecase] Implemented support for the GROUPS frame specification in window functions by adding _sql.over.groups option to _sql.over() and FunctionElement.over(). Pull request courtesy Kaan Dikmen.

    References: #12450

  • [sql] [bug] Fixed issue in CTE constructs involving multiple DDL _sql.Insert statements with multiple VALUES parameter sets where the bound parameter names generated for these parameter sets would conflict, generating a compile time error.

    References: #12363

  • [sql] [bug] Fixed regression caused by #7471 leading to a SQL compilation issue where name disambiguation for two same-named FROM clauses with table aliasing in use at the same time would produce invalid SQL in the FROM clause with two "AS" clauses for the aliased table, due to double aliasing.

... (truncated)

Commits

Dependabot compatibility score

You can trigger a rebase of this PR by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

Note
Automatic rebases have been disabled on this pull request as it has been open for over 30 days.

@rafizamankhan
Copy link
Contributor

@dependabot rebase

@dependabot dependabot bot force-pushed the dependabot/pip/dot-setup/pip/sqlalchemy-2.0.40 branch from de19875 to 34485ec Compare April 5, 2025 03:20
@github-actions github-actions bot added the Abandoned PR - Needs New Owner No activity on PR for more than 2 weeks -- seeking new owner to complete label Apr 20, 2025
@martig7 martig7 removed their assignment Apr 25, 2025
Copy link

codecov bot commented May 28, 2025

Codecov Report

❌ Patch coverage is 42.85714% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 21.72%. Comparing base (26ad49e) to head (add0c57).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main   #11578      +/-   ##
============================================
+ Coverage     20.56%   21.72%   +1.15%     
- Complexity     9282     9303      +21     
============================================
  Files           268      268              
  Lines         35574    35696     +122     
  Branches        457      457              
============================================
+ Hits           7317     7755     +438     
+ Misses        27804    27488     -316     
  Partials        453      453              
Flag Coverage Δ
autograder 21.31% <5.88%> (-0.04%) ⬇️
js 2.09% <ø> (ø)
migrator 100.00% <100.00%> (ø)
php 20.62% <ø> (+1.38%) ⬆️
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 88.88% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor Author

dependabot bot commented on behalf of github Jun 1, 2025

A newer version of sqlalchemy exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.

@williamschen23
Copy link
Contributor

@dependabot recreate

Bumps [sqlalchemy](https://github.com/sqlalchemy/sqlalchemy) from 1.4.29 to 2.0.40.
- [Release notes](https://github.com/sqlalchemy/sqlalchemy/releases)
- [Changelog](https://github.com/sqlalchemy/sqlalchemy/blob/main/CHANGES.rst)
- [Commits](https://github.com/sqlalchemy/sqlalchemy/commits)

---
updated-dependencies:
- dependency-name: sqlalchemy
  dependency-version: 2.0.40
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>
@dependabot dependabot bot force-pushed the dependabot/pip/dot-setup/pip/sqlalchemy-2.0.40 branch from 808dd95 to dca86b7 Compare June 9, 2025 15:16
@williamschen23 williamschen23 changed the title [Dependency] Bump sqlalchemy from 1.4.29 to 2.0.40 in /.setup/pip [Dependency] Bump sqlalchemy from 1.4.29 to 2.0.41 in /.setup/pip Jun 9, 2025
@jeffrey-cordero
Copy link
Contributor

jeffrey-cordero commented Jul 23, 2025

This may require updates to the existing submitty_install script or some sort of system admin logic because I always get the following error, but it's resolved on pip3 install sqlalchemy==2.0.41 - this would also fix the existing failing DB Check test.

Checking for system and database migrations
Running up migrations for master...Traceback (most recent call last):
  File "/usr/local/submitty/GIT_CHECKOUT/Submitty/migration/run_migrator.py", line 9, in <module>
    cli.run(sys.argv[1:], config_path)
  File "/usr/local/submitty/GIT_CHECKOUT/Submitty/migration/migrator/cli.py", line 94, in run
    getattr(main, args.command)(args)
  File "/usr/local/submitty/GIT_CHECKOUT/Submitty/migration/migrator/main.py", line 171, in migrate
    handle_migration(args)
  File "/usr/local/submitty/GIT_CHECKOUT/Submitty/migration/migrator/main.py", line 214, in handle_migration
    migrate_environment(
  File "/usr/local/submitty/GIT_CHECKOUT/Submitty/migration/migrator/main.py", line 323, in migrate_environment
    if database.has_table(database.migration_table.__tablename__):
  File "/usr/local/submitty/GIT_CHECKOUT/Submitty/migration/migrator/db.py", line 102, in has_table
    self.inspector.clear_cache()
AttributeError: 'PGInspector' object has no attribute 'clear_cache'

@williamschen23
Copy link
Contributor

@jeffrey-cordero migrations run before we install the new packages for the checkout. I'm not sure if this is iin the scope of this pr

@jeffrey-cordero
Copy link
Contributor

@jeffrey-cordero migrations run before we install the new packages for the checkout. I'm not sure if this is iin the scope of this pr

Should be in the scope of the PR as DB Check will fail until it's resolved. We should be updating / installing missing dependencies as needed given the migrations require the right version of SQLAlchemy.

@williamschen23
Copy link
Contributor

@jeffrey-cordero migrations run before we install the new packages for the checkout. I'm not sure if this is iin the scope of this pr

Should be in the scope of the PR as DB Check will fail until it's resolved. We should be updating / installing missing dependencies as needed given the migrations require the right version of SQLAlchemy.

db check will fail regardless. It only checks out main's dependencies and will fail regardless of if install is fixed or not. Barb said that it is fine if db check fails

@jeffrey-cordero
Copy link
Contributor

@jeffrey-cordero migrations run before we install the new packages for the checkout. I'm not sure if this is iin the scope of this pr

Should be in the scope of the PR as DB Check will fail until it's resolved. We should be updating / installing missing dependencies as needed given the migrations require the right version of SQLAlchemy.

db check will fail regardless. It only checks out main's dependencies and will fail regardless of if install is fixed or not. Barb said that it is fine if db check fails

Makes sense, but this will require a manual installation to ensure submitty_install works, so the PR description should be updated. The package installation on GitHub Actions will resolve once this is merged into main, but I suppose if any other issues exist (I can't get the workflow running locally through Docker), someone else will address them.

@williamschen23 williamschen23 changed the title [Dependency] Bump sqlalchemy from 1.4.29 to 2.0.41 in /.setup/pip [SYSADMIN ACTION][Dependency] Bump sqlalchemy from 1.4.29 to 2.0.41 in /.setup/pip Jul 23, 2025
Copy link
Contributor

@lavalleeale lavalleeale left a comment

Choose a reason for hiding this comment

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

Code change looks good to me, everything that can be the same is and everything that can't is as close as possible and should have the same functionality

@github-project-automation github-project-automation bot moved this from In Review to Awaiting Maintainer Review in Submitty Development Jul 24, 2025
@williamschen23
Copy link
Contributor

@jeffrey-cordero im still getting an error for sending emails when i do vagrant up. Can you verify if this is happening on your end too? It says that the directory should be chown -R submitty_daemon directory but the first files in there are owned by root.

@jeffrey-cordero
Copy link
Contributor

@jeffrey-cordero im still getting an error for sending emails when i do vagrant up. Can you verify if this is happening on your end too? It says that the directory should be chown -R submitty_daemon directory but the first files in there are owned by root.

This is not occurring on my end. I have no issues releasing emails through send_email.py and send_notification.py. If this has anything to do with the previous commit, make sure you run submitty_install, but if the directories already exist, just delete them before running the command, just in case.

image

Copy link
Contributor

@jeffrey-cordero jeffrey-cordero left a comment

Choose a reason for hiding this comment

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

The changes implemented for the update to SQLAlchemy v2 look solid based on the recommended migration strategy. The only inconsistencies I could spot were related to fixing existing errors or removing old columns that are no longer referenced in the respective tables. The setup, database, and shared binary scripts behave as expected after a fresh vagrant up.

@automateprojectmangement automateprojectmangement bot moved this from Awaiting Maintainer Review to In Review in Submitty Development Jul 25, 2025
bmcutler added a commit to Submitty/submitty.github.io that referenced this pull request Jul 25, 2025
When running `INSTALL_SUBMITTY.sh`, we run the database migration before
updating the python packages. This causes the script to always fail,
hence why we need to install the python packages before we run
`INSTALL_SUBMITTY.sh`

Submitty/Submitty#11578

---------

Co-authored-by: Barb Cutler <[email protected]>
Co-authored-by: Barb Cutler <Barb Cutler>
@bmcutler bmcutler merged commit 3ab6491 into main Jul 25, 2025
27 of 28 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in Submitty Development Jul 25, 2025
@bmcutler bmcutler deleted the dependabot/pip/dot-setup/pip/sqlalchemy-2.0.40 branch July 25, 2025 17:31
bmcutler pushed a commit that referenced this pull request Jul 27, 2025
### Why is this Change Important & Necessary?
<!-- Include any GitHub issue that is fixed/closed using "Fixes
#<number>" or "Closes #<number>" syntax.
Alternately write "Partially addresses #<number>" or "Related to
#<number>" as appropriate. -->
Regression came from #11578 
Current production is broken right now with team gradeables. This is
because the update portion of the team gradeable does not have a
`connection.commit()`, which the non-team gradeable has. Putting the
commit statement outside of both if and else if is going to fix the
issue.

Whenever you submit a team gradeable, you will see the `Something went
wrong will this submission` banner. When you take a closer look in the
`electronic_gradeable_data` table, the value of `completed` will always
be false. We can manually check the grades that we received in
`results/grades.txt` to see that we actually received a grade for the
assignment. As mentioned before, this is because the table is not being
updated when the job finishes, and thus the user recieves the error.

### What is the New Behavior?
<!-- Include before & after screenshots/videos if the user interface has
changed. -->
Putting the commit statement outside of both if and else if is going to
fix the issue. This ensures that insertions and updates will both be
commited.
The autoload_with=engine vs db is a semantic difference as it doesnt
impact the overall issue. SQLAlchemy takes both conection and engine,
but as engine was more widely used and recommended in sqlalchemy, we
should use this instead.

### What steps should a reviewer take to reproduce or test the bug or
new feature?

### Automated Testing & Documentation
<!-- Is this feature sufficiently tested by unit tests and end-to-end
tests?
If this PR does not add/update the necessary automated tests, write a
new GitHub issue and link it below.
Is this feature sufficiently documented on submitty.org?
Link related PRs or new GitHub issue to update documentation. -->
We should do a cypress autograding test of team gradeables so that it
doesnt get broken in the future.

### Other information
<!-- Is this a breaking change?  
Does this PR include migrations to update existing installations?  
Are there security concerns with this PR? -->
check_refresh for team gradeables is broken. It will need to be fixed
before we make the autograding test, as cypress relies on the automatic
refreshs in order to detect new elements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants