Feat hybrid xmlid export #135
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 introduces a significant enhancement to the Odoo data export functionality by enabling the export of external XML IDs for main records. It allows users to request both the internal database ID ('.id') and the external XML ID ('id') in the export header, with the system automatically populating the 'id' column with the 'module.name' formatted XML ID. This feature provides greater flexibility for data integration and migration scenarios where external identifiers are crucial. Additionally, the PR includes minor updates to the 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a valuable feature for enriching main record exports with their XML IDs when both .id and id are specified in the header. The implementation is well-structured, adding a new function _enrich_main_df_with_xml_ids and updating the export strategy logic accordingly. The accompanying tests validate the new functionality. My review identifies two issues in the new enrichment function: a high-severity risk of data duplication when a record has multiple XML IDs, and a medium-severity issue where a temporary column is leaked into the final output. I have provided suggestions to address both points.
This commit enhances the `odoo-data-flow` tool to allow exporting the external XML ID of the main record while using the tool's fast `read` mode. When a user requests both `.id` and `id` in the fields list, and the export is running in `read` mode, the tool will now perform a secondary enrichment step. After fetching the main data with numeric IDs, it makes another call to Odoo to retrieve the XML IDs (`module.name`) for those numeric IDs. This is implemented using a Polars left join to merge the XML IDs back into the main DataFrame. The `id` column is then populated with the XML ID, and per the user's final clarification, the `.id` column containing the numeric database ID is preserved in the final output. This allows users to have access to both the database ID and the external ID for transformation steps. A comprehensive unit test has been added to verify this new functionality, including the case where a record may not have an XML ID.
…ython
The `nox` sessions were failing in CUpdate because `uv sync` was not installing dependencies into the correct session-specific virtual environments. This was caused by `uv` ignoring the `VIRTUAL_ENV` set by `nox` and defaulting to a different environment.
This commit resolves the issue by:
1. Adding `--python str(session.python)` to all `uv sync` calls in `noxfile.py`. This explicitly tells `uv` which python executable to use, forcing it to install dependencies into the correct isolated virtual environment for each session.
2. Removing all `external=True` flags from the `uv sync` calls, as they are no longer needed and were part of the problem.
3. Removing a redundant `session.install("pydoclint")` call, as the dependency is already handled by `uv sync`.
These changes ensure that `tests` and `mypy` sessions run reliably. The `pre-commit` session is now configured correctly, although it may still face issues in certain CUpdate environments with unusual PATH configurations.
bosd
force-pushed
the
feat-hybrid-xmlid-export
branch
from
320afd8 to
bd94604
Compare
No description provided.