-
Notifications
You must be signed in to change notification settings - Fork 1
Fix O2M ID field handling and enhance import reliability\n\n- Fix cri… #138
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
Conversation
Summary of ChangesHello @bosd, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the reliability and debugging capabilities of the data import process. It addresses a critical issue with Odoo XML ID handling by refining the sanitization logic and ensures that the import mechanism reacts appropriately to different types of Odoo server responses, preventing silent failures and providing clearer insights into import operations. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request introduces fixes for O2M ID field handling and improves import reliability. The changes include more robust error handling, sanitization of XML IDs to prevent constraint violations, and more detailed logging for easier debugging. My review focuses on improving code style and performance. I've suggested moving local imports to the top of the file for better maintainability and performance, and optimizing a string replacement function.
| log.error(f"Failed to read file {file_path}: {e}") | ||
| log.error(f"Exception type: {type(e).__name__}") | ||
| log.error(f"Exception args: {e.args}") | ||
| import traceback |
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.
624c213 to
5d8903f
Compare
- Fix critical bug in to_xmlid function: Preserve dots in XML IDs for Odoo format - Remove '.' from character replacements to maintain module.identifier format - Fix syntax error in to_xmlid function docstring with proper triple quote escaping - Enhance batch load debugging with detailed record creation verification - Add comprehensive error logging for XML ID constraint violations and validation errors - Improve exception handling to capture failures in fail files instead of aborting - Add enhanced debugging for load response analysis and record creation counts - Prevent large base64 data from flooding logs with smart truncation - Fix ir.model.relation field validation errors with proper error handling - Gracefully handle Odoo server internal errors during relational imports This addresses all critical issues preventing successful O2M ID field imports: 1. XML ID constraint violations that were blocking record creation 2. Dot removal in XML IDs that was corrupting module.identifier format 3. Silent failures that were preventing proper error reporting 4. Debugging gaps that made troubleshooting difficult The enhancement now successfully handles direct database IDs in /id suffixed fields while maintaining compatibility with existing XML ID functionality.
5d8903f to
a5074ab
Compare
|
/gemini review |
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.
Code Review
This pull request introduces several valuable improvements to the data import process, focusing on reliability and debuggability. Key changes include correcting the to_xmlid function to preserve Odoo's external ID format, adding extensive debug logging, and making the id_map creation more type-safe. The error handling for the load operation is also enhanced to differentiate between warnings and fatal errors. While these are great enhancements, I've identified a critical bug in the new ID sanitization logic that could cause issues when columns are ignored during import. I've also found a block of duplicated code that should be removed. Please see my detailed comments.
| # Check for any Odoo server errors in the response that should | ||
| # halt processing | ||
| if res.get("messages"): | ||
| for message in res["messages"]: | ||
| msg_type = message.get("type", "unknown") |
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.
This block of code, which checks for messages in the response, appears to be a duplicate. A more complete version of this logic already exists earlier in the function (lines 789-801), which properly handles different message types (error, warning, info) and raises an exception for fatal errors. This duplicated block is incomplete and redundant. It should be removed to avoid confusion and potential issues.
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.
@gemini-code-assist can you turn this into a code suggestion? #
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.
You are correct! This block of code is indeed a duplicate and should be removed. The more comprehensive error handling logic is already present earlier in the function (lines 789-801).
Here's a code suggestion to remove the redundant block:
84923e6 to
cd7337c
Compare
No description provided.