Release v2.7.2 #7132
Release v2.7.2 #7132
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## support/2.7.x #7132 +/- ##
=================================================
+ Coverage 79.13% 79.16% +0.04%
=================================================
Files 565 565
Lines 43391 43488 +97
=================================================
+ Hits 34331 34425 +94
- Misses 9060 9063 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I am bit confused by the number of commits (for a patch release!) shouldn't we just pick the most critical bug fixes and leave the rest for the 2.8.0). That seems much safer and in line with other patch release which have always been very targeted in the past. (EDIT: sorry if I missed discussion about this) |
Hm, that's a valid concern... though, I actually went manually through all the commits, and only selected the ones that are suitable for a patch release following SemVer (that is, no new features). E.g., numpy upgrade, click upgrade, unstashing, any new methods, etc., are not included. In addition, the recent PRs have been rather small, e.g., I already have two one-line change PRs to drop the duplicated CLI arguments. We also plan v2.8.0, with a bunch of new features already in the pipeline, rather soon. So, I do think it's fine to go through with this patch release (given that now it's ready, basically). What do you think? EDIT: Didn't know about the history of patch releases, and how targeted they were in the past, tbh. Though, my original motivation for the patch release was fixing the PSQL OpErrs that would lead to archive export failures, and as that kept dragging on, and people reported (and fixed) more and more issues, the list of things in the 2.7.2 project grew ^^ Thoughts, @khsrali, @agoscinski? |
|
Well, the fact the 2.8 release is imminent is more of an argument to have a small and targeted patch release only containing critical bugfixes.
To be super clear, I fully trust you did a great job of this. But in the end it is just statistics --- every change, (even a "bugfix") brings an extra chance of breaking something. That's why I think patch releases should be targeted only for critical bugfixes. To me in this case those are the async-ssh fixes and your PSQL import/export fixes (perhaps others I am not aware of).
Yeah, I think in semver it doesn't say that every bugfix need to be released in a patch release --- it can very well be in minor release. It says the opposite implication --- patch releases can only contain bugfixes and nothing else. |
|
Anyway, this is obviously up to you, just wanted to flag this. :-) btw: It looks like 2.7.1 is missing from CHANGELOG? |
|
Cheers, thanks for bringing this up, @danielhollas I thought more about it on the way home and I think you're right, better to have a smaller, more focused patch release 🙏🏻 Will take care of it on Monday! |
GeigerJ2
force-pushed
the
release/2.7.2
branch
from
871b8fa to
a9374b6
Compare
GeigerJ2
force-pushed
the
release/2.7.2
branch
from
a9374b6 to
adf6fc0
Compare
…ry detection (#6935) Previously, the configuration directory lookup split the AIIDA_PATH environment variable using a hardcoded ':' separator, which caused issues on Windows systems where ';' is used as the path separator. This update introduces platform detection using sys.platform to select the correct separator for splitting paths, ensuring compatibility across operating systems. --------- Co-authored-by: lainme <[email protected]> Co-authored-by: Ali Khosravi <[email protected]> (cherry picked from commit 32d515a)
(cherry picked from commit 5e4da5b)
move values to insert to `execute` statement rather than `insert` (cherry picked from commit 9bccdc8)
(cherry picked from commit 019172c)
(cherry picked from commit 3a7d440)
|
Starting over again with less commits, and the PR descriptions non fucked up |
This fixes SQLAlchemy `OperationalError`s when creating large archives with the PSQL backend by batching query filters using `DEFAULT_FILTER_SIZE` (999). The default value was re-used from PR #6889 in which it was originally set to avoid exceeding the `SQLITE_MAX_VARIABLE_NUMBER` limit for archive imports with the SQLite backend. Key changes: - Add `filter_size` parameter to `create_archive()`, `_collect_required_entities()` and all helper functions called therein to apply batching to query filters, using the default value of 999 (not exposed to the user via CLI) - Move `batch_iter()` to `aiida.common.utils` for reuse - Drop `QueryParams` in favor of the explicit `batch_size` and `filter_size` arguments - Update graph traversal to batch node ID queries, as well Some import functions still need batching (see #6907), which will be done in a follow-up PR. --------- Co-authored-by: Daniel Hollas <[email protected]> (cherry picked from commit cfbbd68)
To avoid the command being seemingly stuck. (cherry picked from commit fc00f5d)
For table construction, the pydantic `FieldInfo`s of each model field are being used to exclude fields with `exclude_from_cli=True`. In addition, `is_attribute` is being (mis)used (see comment in src) to avoid bare `AtrributeError` exception handling, which otherwise captures `filepath_files` of `PortableCode`, which is never set on the instance or stored in the database after creation of the entity. The meaningless `test_code_show` test function has been improved and regression tests using the `file_regression` fixture were added that compare the command output to the expected output, stored in `txt` files. This ensures no other formatting changes appear in the future (no duplications, wrong capitalization, etc., as resolved by this PR). (cherry picked from commit 32742c0)
This is required if it's in the top folder (of the repository) and will thus be put in the top folder of the SCRATCH directory (AiiDA's RemoteData), otherwise the bash script (given to the scheduler) will not be able to execute it, since `./` is not typically on the bash's PATH environment variable for security reasons. Also, fixing a small bug where the PortableCode will not accept using a binary defined in a subfolder of the Repository. (cherry picked from commit 9256f2f)
Before this fix, those codes were not shown (e.g., PortableCodes). Now, they are properly added to the list. (cherry picked from commit 27b52da)
* CI: Add Slack notification to install-with-conda-job (cherry picked from commit 0dcd10a)
This should improve performance of async_ssh plugin for lots of small files since we're no longer sleeping 500ms when waiting for a lock. Also makes the code more safe since one doesn't need to manually lock and unlock, and fixes a bug since the number of file IO connections was not decreased when an OsError exception was thrown. (cherry picked from commit ad5cafd)
Use database-specific optimizations for large IN clauses in QB, reducing parameter usage from N parameters (one per value) to 1 parameter per list. This avoids hitting database parameter limits (SQLite: ~999, PostgreSQL: ~65k) when filtering on large sets of IDs. Implementation details: PostgreSQL uses `unnest()` with native ARRAY types. The list is passed as a single array parameter using `type_coerce()` (a Python-side SQLAlchemy hint that doesn't generate SQL CAST). PostgreSQL natively understands arrays, so values from `unnest()` are already properly typed. SQLite uses `json_each()` since it lacks native array support. The list is serialized to JSON and passed as a single parameter. Since `json_each()` returns all values as TEXT, explicit SQL CAST is required to convert to the target column type. For very large lists (>500k items), automatic batching splits them into multiple OR'd conditions. The recursion depth is always exactly 2. The first call splits into ≤500k batches, and recursive calls immediately hit the base case since each batch is guaranteed to be ≤500k items. This optimization eliminates the need for `filter_size` parameter batching throughout the codebase, particularly in archive import/export operations where large node sets are common. (cherry picked from commit 8d562b4)
### Problem The JobsList class had a race condition where: 1. Job A requests a status update, triggering a scheduler query 2. While the scheduler is being queried (with only Job A), Job B also requests an update 3. After the scheduler returns (with only Job A's status), both futures were resolved 4. Job B's future was resolved as DONE, because AiiDA assumes any job ID that disappeared from the scheduler query has completed This premature "DONE" status causes several critical issues: - Premature retrieval: AiiDA attempts to retrieve output files while the job is still running - Corrupted files: Files may be incomplete or still being written when retrieved - False failure reports: Jobs still running may be incorrectly marked as failed This issue only surfaces when using async transport plugins like core.ssh_async or aiida-firecrest, where the timing conditions make the race condition more likely to occur. ### Solution Only resolve futures for jobs that were actually inspected by the scheduler ### Testing Added test_prevent_racing_condition which explicitly tests the race condition scenario (cherry picked from commit e79f0a4)
The removal of the filter_size batching in #6998 caused the progress bar to only update once at the end instead of incrementally. Here, we re-introduce query batching (50k IDs per batch for UX) and added QueryBuilder streaming-based progress updates (every ~10k entities) by replacing the blocking list comprehension with an explicit for-loop. Progress now updates during both query batching (for UX) and result streaming. (cherry picked from commit 4e54cd4)
GeigerJ2
force-pushed
the
release/2.7.2
branch
2 times, most recently
from
046da5e to
49d0614
Compare
| Note: The 500k batch threshold is chosen to balance several factors: parameter limits | ||
| (each batch uses 1 parameter, allowing SQLite's 999-parameter limit to handle up to | ||
| ~500M items and PostgreSQL's 65k-parameter limit to handle up to ~33B items), memory | ||
| constraints (in practice, Python memory becomes the bottleneck as a list of 500M items | ||
| would require 4-20GB RAM before reaching the database), and database performance | ||
| (modern databases handle 500k-item arrays/JSON easily on typical workstations and servers). |
There was a problem hiding this comment.
Re-wrote this as a paragraph, as RTD was always failing with the previous text, complaining about the formatting of the lists (even when adding blank lines, I spent like an hour trying to resolve these complaints...):
- "Bullet list ends without a blank line; unexpected unindent"
- "Definition list ends without a blank line; unexpected unindent"
As I don't think this really matters, I re-wrote it as a non-rst plain text paragraph.
There was a problem hiding this comment.
Would it make sense to merge this change separately to main first?
There was a problem hiding this comment.
Hm, yeah, that does make sense. I just realized that the offending RST code in the docstring might've passed CI on main as autodoc was being dropped before in #7056, so there was never a proper RST "compilation" step, and the issue went unnoticed. I went ahead and fixed it to use actual code directives for the SQL statements, so that RST doesn't mis-interpret them, and give misleading exceptions. Will open a PR, that should be possible to merge and cherry-pick here soon.
GeigerJ2
force-pushed
the
release/2.7.2
branch
2 times, most recently
from
65698c2 to
4b941ab
Compare
| from __future__ import annotations | ||
|
|
|
Wow, OK, this process turned out to be more painful than I anticipated 😅 In addition, I added commit 4b941ab to make CI pass. Reasons for each change are in the comments above, @danielhollas, see here. Now just missing a few final steps for tomorrow:
:) |
GeigerJ2
requested review from
danielhollas and
khsrali
and removed request for
agoscinski and
unkcpz
danielhollas
left a comment
danielhollas
left a comment
There was a problem hiding this comment.
In addition, I added commit 4b941ab to make CI pass. Reasons for each change are in the comments above, @danielhollas, see here.
All looks good, thanks!
Now just missing a few final steps for tomorrow: release commit, pushing to test-pypi, adding the tag, actually publish to pypi, and announcements :)
You also need to add CHANGELOG right?
GeigerJ2
force-pushed
the
release/2.7.2
branch
from
4b941ab to
4fe55b9
Compare
(cherry picked from commit cc0bb48)
GeigerJ2
force-pushed
the
release/2.7.2
branch
from
4fe55b9 to
4005693
Compare
|
Thanks for the assistance with this, @danielhollas 🫶
Yes, that's part of the release commit :) |
GeigerJ2
force-pushed
the
release/2.7.2
branch
from
3af1e84 to
4aba812
Compare
GeigerJ2
force-pushed
the
release/2.7.2
branch
from
4aba812 to
fb038ac
Compare
GeigerJ2
force-pushed
the
release/2.7.2
branch
from
fb038ac to
2befa6b
Compare
Ping @khsrali @agoscinski