Skip to content
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

DRIVERS-3002 Use mongodl.py in download-mongodb.sh with starts-with version comparison #529

Merged
merged 8 commits into from
Oct 24, 2024

Conversation

eramongodb
Copy link
Contributor

@eramongodb eramongodb commented Oct 24, 2024

Reattempt of #525.

Extends the version comparison operator in the DB query of matching download entries from an exact string comparison (version=:version) to include a "starts-with" pattern match expression using the LIKE operator. This permits version="5.1" to match "5.1.Z", but it will not match a hypothetical "X.5.0" or "5.10.Z". Similarly, version="5" will match version="5.Y.Z", but will not match a hypoethetical "50.Y.Z". A full version number version=5.1.0 will still match "5.1.0" exactly using the existing exact comparison operator.

The extra version_pattern=f'{version}.% argument is necessary to workaround syntax errors when the % is embedded in the query string itself, e.g. writing version=:version OR version LIKE :version.% in the query string itself raises an exception (various attempts to escape the % character failed):

  File ".evergreen/mongodl.py", line 324, in __call__
    return self._cursor.execute(query, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.OperationalError: near ")": syntax error

@eramongodb
Copy link
Contributor Author

The "starts-with" pattern also addresses the handling of version numbers without a patch version number (e.g. 5.0). The ORDER BY version query operator selects the newest patch version number available given the query parameters. The SUPPORTED_VERSIONS injection is therefore no longer necessary.

Copy link
Member

@blink1073 blink1073 left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@blink1073 blink1073 changed the title mongodl: use starts-with pattern when matching version strings DRIVERS-3002 mongodl: use starts-with pattern when matching version strings Oct 24, 2024
@eramongodb eramongodb changed the title DRIVERS-3002 mongodl: use starts-with pattern when matching version strings DRIVERS-3002 Use mongodl.py in download-mongodb.sh with starts-with version comparison Oct 24, 2024
@eramongodb
Copy link
Contributor Author

eramongodb commented Oct 24, 2024

Some error messages added in 9142b70#diff-10255c591aa6273801d14cd261ba71498474b9b21ba02aee1df67e6e9cd60a2fR470 (relating to the DISTRO_ID_TO_TARGET map) were not being printed to stderr, thus leading to the resulting error message being treated as part of the download URL. Failure to account for word splitting in expansions of the resulting Bash variable(s) in subsequent curl commands resulted in some awful behavior. Both issues have been fixed. The current list of missing targets is now output properly (+ nicer formatting) without affecting the contents of the download URL.

The missing DISTRO_ID_TO_TARGET entries (and correponding failure of test-mongodl-full tasks) may be addressed by a followup PR.

@eramongodb eramongodb requested a review from blink1073 October 24, 2024 20:27
@blink1073
Copy link
Member

New error:

[2024/10/24 15:41:55.151] Missing targets in DISTRO_ID_TO_TARGET:
[2024/10/24 15:41:55.151]  - linux_i686
[2024/10/24 15:41:55.151]  - linux_x86_64
[2024/10/24 15:41:55.151]  - osx

@blink1073
Copy link
Member

I think they need to be added to this list:

if not found and target not in ['macos', 'windows']:

@blink1073
Copy link
Member

Ah, I see your comment. I agree, let's merge and iterate. Thank you!

@eramongodb eramongodb merged commit e6a901d into mongodb-labs:master Oct 24, 2024
57 of 60 checks passed
@eramongodb eramongodb deleted the det-mongodl branch October 24, 2024 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants