Skip to content

[5192][ADD] openai_connector, product_name_generator - #161

Merged
yostashiro merged 61 commits into
15.0from
5192-add-openai_connecter
Oct 8, 2025
Merged

[5192][ADD] openai_connector, product_name_generator #161
yostashiro merged 61 commits into
15.0from
5192-add-openai_connecter

Conversation

@nobuQuartile

Copy link
Copy Markdown
Contributor

@nobuQuartile
nobuQuartile force-pushed the 5192-add-openai_connecter branch from f1b788c to 4b7b5fa Compare June 9, 2025 08:35
@nobuQuartile nobuQuartile changed the title [5192][ADD] openai_connecter [5192][ADD] openai_connector Jun 9, 2025
@nobuQuartile
nobuQuartile marked this pull request as draft June 9, 2025 08:52
@nobuQuartile
nobuQuartile force-pushed the 5192-add-openai_connecter branch from 4b7b5fa to 8ff032c Compare June 9, 2025 09:15
@nobuQuartile nobuQuartile changed the title [5192][ADD] openai_connector [5192][ADD] openai_connector, product_name_creater Jun 11, 2025
@AungKoKoLin1997

Copy link
Copy Markdown
Contributor

@nobuQuartile Please test the product_name_generator module. I tried to add the button in the footer of extra product media view but there is a issue when I used the js code. So, I just add in the header at the moment.

Comment thread product_name_generator/models/product_image.py Outdated
@nobuQuartile nobuQuartile changed the title [5192][ADD] openai_connector, product_name_creater [5192][ADD] openai_connector, product_name_generator Jun 16, 2025

@AungKoKoLin1997 AungKoKoLin1997 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Partial Review.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you please create new PR to run the pre-commit?
So, this file will not include in this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done
#163

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have rebased this branch.

Comment thread openai_connector/__manifest__.py
Comment thread openai_connector/__manifest__.py Outdated
Comment thread openai_connector/__manifest__.py Outdated
Comment thread openai_connector/models/openai_vision_session.py Outdated
Comment thread openai_connector/models/openai_vision_session.py Outdated
api_key = self.env.company.openai_api_key
if not api_key:
raise UserError(_("OpenAI API key is not configured"))
client = OpenAI(api_key=api_key)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this correct variable name?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I changed it to openai_client.
Is this good? How do you feel?

Comment thread openai_connector/models/openai_vision_session.py Outdated
Comment thread product_name_generator/models/product_image.py Outdated
Comment thread product_name_generator/models/product_image.py Outdated
@nobuQuartile
nobuQuartile force-pushed the 5192-add-openai_connecter branch from 84cc478 to 7142490 Compare June 18, 2025 06:34
@nobuQuartile

Copy link
Copy Markdown
Contributor Author

@AungKoKoLin1997
I have updated what you reviewed. Could you please continue reviewing this PR?

Comment thread openai_connector/__manifest__.py Outdated
Comment thread openai_connector/__manifest__.py Outdated
Comment thread openai_connector/models/__init__.py
Comment thread openai_connector/models/openai_vision_session.py
Comment thread openai_connector/models/openai_vision_session.py Outdated
Comment thread openai_connector/models/openai_vision_session.py Outdated
@AungKoKoLin1997

Copy link
Copy Markdown
Contributor

@nobuQuartile Is this PR ready to review?

@AungKoKoLin1997

Copy link
Copy Markdown
Contributor

@nobuQuartile Please review my last commit. I make some adjustments for openai_connector. Please update the readme with clear information and better to add USAGE.md with the instruction of how to use this module from another modules.
Now, the functionality of product_name_generator is working.

Comment thread product_name_generator/models/product_template.py

@yostashiro yostashiro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Partial review.

Comment thread openai_connector/models/openai_vision_session.py Outdated
Comment thread openai_connector/models/openai_vision_session.py Outdated
Comment thread openai_connector/models/openai_vision_session.py Outdated
Comment thread openai_connector/models/res_company.py Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The file name should be xxx_views.xml.

Indentation should be fixed. Same for other .xml files.

Comment thread product_name_generator/__manifest__.py Outdated
@AungKoKoLin1997

Copy link
Copy Markdown
Contributor

I feel like using image_url is not a good practice. If we want to use this module for another project or later this project doesn't use website module anymore, this module will not be worked. But as per nobu research, we can't use binary data because of large data than openai can accept. So, we just forget above points for now.

@nobuQuartile
nobuQuartile force-pushed the 5192-add-openai_connecter branch from db72266 to 2735665 Compare July 7, 2025 02:57
@AungKoKoLin1997
AungKoKoLin1997 force-pushed the 5192-add-openai_connecter branch from 2735665 to f6aacf8 Compare July 7, 2025 03:08
@AungKoKoLin1997
AungKoKoLin1997 force-pushed the 5192-add-openai_connecter branch from 3435850 to 292a5db Compare July 17, 2025 04:22
@AungKoKoLin1997

Copy link
Copy Markdown
Contributor

@nobuQuartile Check the latest changes and test it.
I think the readme of openai connector is not good enough especially CONFIGURE.md.
Please improve it when you have time.

@nobuQuartile

Copy link
Copy Markdown
Contributor Author

@AungKoKoLin1997
Could you create the data of base_api_connection in openai_connecter?

@AungKoKoLin1997

Copy link
Copy Markdown
Contributor

Could you create the data of base_api_connection in openai_connecter?

I don't think that is needed. It is described in the CONFIGURE.md

Comment thread openai_connector/readme/CONFIGURE.md Outdated
Comment thread product_name_generator/models/product_template.py Outdated
@nobuQuartile
nobuQuartile force-pushed the 5192-add-openai_connecter branch from 8129a5d to 3e9bcaa Compare July 17, 2025 10:15
@nobuQuartile

Copy link
Copy Markdown
Contributor Author

I think the error will be fixed after the base_api_connection's PR is merged and this branch is rebased.

Comment thread openai_connector/models/openai_vision_session.py Outdated
@AungKoKoLin1997

Copy link
Copy Markdown
Contributor

@nobuQuartile Please check my last commit.

@nobuQuartile

Copy link
Copy Markdown
Contributor Author

@nobuQuartile Please check my last commit.
Good. Thank you.

@nobuQuartile
nobuQuartile requested review from yostashiro and removed request for yostashiro September 18, 2025 06:24

@yostashiro yostashiro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just some style adjustments. Otherwise, LGTM.

Comment thread product_name_generator/data/ir_config_parameter.xml Outdated
Comment thread product_name_generator/models/product_template.py Outdated
@nobuQuartile
nobuQuartile force-pushed the 5192-add-openai_connecter branch from 8b437a1 to f389c82 Compare September 25, 2025 05:59
@nobuQuartile

Copy link
Copy Markdown
Contributor Author

I have force-pushed to solve the pre-commit error.

@nobuQuartile

nobuQuartile commented Oct 2, 2025

Copy link
Copy Markdown
Contributor Author

I did the functional test in the test env.
We can merge this PR after verifying that the PR test error does not occur once the related PR is merged.

@nobuQuartile

Copy link
Copy Markdown
Contributor Author

Could you merge this PR?
I just wanted to let you know that I'll do this today.

@nobuQuartile

Copy link
Copy Markdown
Contributor Author

Sorry, this one should be merged after the base_api_connector and rebase this branch.

@nobuQuartile

Copy link
Copy Markdown
Contributor Author

@AungKoKoLin1997
Do you know why "test with OCB" has an error?

@AungKoKoLin1997

Copy link
Copy Markdown
Contributor

@AungKoKoLin1997 Do you know why "test with OCB" has an error?

@nobuQuartile Please add base_api_connection in text-requirements.txt.

@nobuQuartile
nobuQuartile force-pushed the 5192-add-openai_connecter branch 3 times, most recently from a39d971 to a6c53cc Compare October 8, 2025 07:35
@nobuQuartile
nobuQuartile force-pushed the 5192-add-openai_connecter branch from a6c53cc to 234b43a Compare October 8, 2025 07:37
@nobuQuartile

Copy link
Copy Markdown
Contributor Author

@AungKoKoLin1997
Could you fix the pre-commit error?
I did pre-commit in my env, but it does not fix the error.

@AungKoKoLin1997

Copy link
Copy Markdown
Contributor

@AungKoKoLin1997 Could you fix the pre-commit error? I did pre-commit in my env, but it does not fix the error.

I don't see there is a pre-commit issue in the CIs.
What do you want me to fix?

@nobuQuartile

Copy link
Copy Markdown
Contributor Author

Please forget that, Sorry.

@nobuQuartile

Copy link
Copy Markdown
Contributor Author

Ready to merge

@yostashiro
yostashiro merged commit bc23a3b into 15.0 Oct 8, 2025
2 checks passed
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.

4 participants