-
Notifications
You must be signed in to change notification settings - Fork 557
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
Add Ubuntu 24.04 fiftyone-db
support
#4533
Conversation
WalkthroughThe recent update introduces support for Ubuntu 24.04 by adding MongoDB download links for this version in Changes
Assessment against linked issues
Poem
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range and nitpick comments (4)
package/db/setup.py (4)
Line range hint
138-138
: Avoid using bareexcept
.- except: + except Exception as e: + # Handle specific exceptions or log themThis change will help in identifying and handling specific exceptions more effectively.
Line range hint
154-155
: Use format specifiers for string formatting.- "Release version does not match version: %s and %s" - % (version, VERSION) + f"Release version does not match version: {version} and {VERSION}"This change adheres to modern Python practices and improves code readability.
Line range hint
166-168
: Convert lambda to a function definition for clarity.- is_platform = lambda sys, isa=None: sys == SYSTEM and ( - not isa or isa == MACHINE - ) + def is_platform(sys, isa=None): + return sys == SYSTEM and (not isa or isa == MACHINE)This change enhances readability and maintainability of the code.
Line range hint
237-239
: Enhance exception handling withraise ... from
.- raise IOError( - "Could not find %r in MongoDB archive" % filename - ) + raise OSError( + f"Could not find {filename} in MongoDB archive" + ) from NoneThis change clarifies the flow of exceptions and uses the more modern
OSError
.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- package/db/setup.py (2 hunks)
Additional context used
Ruff
package/db/setup.py
138-138: Do not use bare
except
(E722)
154-155: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
166-168: Do not assign a
lambda
expression, use adef
(E731)Rewrite
is_platform
as adef
216-216: Do not use bare
except
(E722)
237-239: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling (B904)
237-237: Replace aliased errors with
OSError
(UP024)Replace
IOError
with builtinOSError
238-238: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
255-257: Within an
except
clause, raise exceptions withraise ... from err
orraise ... from None
to distinguish them from errors in exception handling (B904)
255-255: Replace aliased errors with
OSError
(UP024)Replace
IOError
with builtinOSError
256-256: Use format specifiers instead of percent format (UP031)
Replace with format specifiers
272-272: Unnecessary open mode parameters (UP015)
Remove open mode parameters
Additional comments not posted (2)
package/db/setup.py (2)
146-146
: Updated version constant to 1.1.4.
Line range hint
216-216
: Avoid using bareexcept
.
"24": { | ||
"aarch64": "https://fastdl.mongodb.org/linux/mongodb-linux-aarch64-ubuntu2204-7.0.4.tgz", | ||
"x86_64": "https://fastdl.mongodb.org/linux/mongodb-linux-x86_64-ubuntu2204-7.0.4.tgz", | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip
Codebase Verification
The provided links are for Ubuntu 22.04, not 24.04.
The MongoDB download links in the setup.py file point to Ubuntu 22.04, as there are no references to Ubuntu 24.04 in the repository. This suggests that MongoDB does not yet officially support Ubuntu 24.04, and the links for Ubuntu 22.04 might have been used as placeholders.
package/db/setup.py
: Lines 98-101
Analysis chain
Added Ubuntu 24.04 support with MongoDB download links.
The links provided are for Ubuntu 22.04, not 24.04. This might be due to MongoDB not yet supporting Ubuntu 24.04. Please confirm if these are the intended links or if it's a placeholder until official support is released.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check MongoDB official support for Ubuntu 24.04.
# Test: Search for official MongoDB release for Ubuntu 24.04. Expect: Confirmation of the release or lack thereof.
rg --type markdown $'Ubuntu 24.04' README.md
Length of output: 45
Script:
#!/bin/bash
# Search for any references to Ubuntu 24.04 in the repository to gather more information.
rg 'Ubuntu 24.04'
# Verify the MongoDB download links in the setup.py file to ensure they are correct.
rg 'mongodb-linux' package/db/setup.py
Length of output: 2933
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- .github/workflows/build-db.yml (1 hunks)
Additional comments not posted (2)
.github/workflows/build-db.yml (2)
Line range hint
73-76
: Ensure the upload path for wheel artifacts is correct.The upload path for non-
sdist
platforms remains unchanged, uploading.whl
files. Verify that this path correctly reflects the intended behavior and includes all necessary artifacts.Verification successful
Line range hint
73-76
: Verified upload path for wheel artifacts.The artifact upload path
package/db/dist/*.whl
is correctly specified for non-sdist
platforms, ensuring that.whl
files are uploaded as intended.
- Path confirmed:
package/db/dist/*.whl
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the artifact upload path for non-`sdist` platforms. # Test: Ensure only `.whl` files are uploaded for non-`sdist` platforms. rg --type yaml $'path: package/db/dist/*.whl'Length of output: 46
Script:
#!/bin/bash # Description: Verify all artifact upload paths in the build-db.yml file. # Test: Find all lines containing 'path:' to check artifact upload paths. rg --type yaml 'path:' .github/workflows/build-db.ymlLength of output: 184
69-72
: Verify the artifact upload path forsdist
platform.The upload path for the
sdist
platform has been modified to upload only.tar.gz
files. Ensure this change correctly reflects the intended behavior and does not miss any necessary artifacts.Verification successful
Verified the artifact upload path for
sdist
platform.The modification to upload only
.tar.gz
files for thesdist
platform is correctly implemented and does not miss any necessary artifacts.
.tar.gz
files are correctly uploaded forsdist
platform..whl
files are uploaded for other platforms.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the artifact upload path for the `sdist` platform. # Test: Ensure only `.tar.gz` files are uploaded for the `sdist` platform. rg --type yaml $'path: package/db/dist/*.tar.gz'Length of output: 49
Script:
#!/bin/bash # Description: Search for all `path` directives within the `.github/workflows/build-db.yml` file to gather more context. rg --type yaml 'path:' .github/workflows/build-db.ymlLength of output: 184
Script:
#!/bin/bash # Description: Extract the surrounding context of each `path` directive within the `.github/workflows/build-db.yml` file to verify the correctness of the change. rg --type yaml -A 3 'path:' .github/workflows/build-db.ymlLength of output: 528
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me
What changes are proposed in this pull request?
Resolves #4526
How is this patch tested? If it is not, please explain why.
Confirmed functionality in #4526 discussion
What areas of FiftyOne does this PR affect?
fiftyone
Python library changesSummary by CodeRabbit