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

Project 1: AI Chatbot for Whole Website #2254

Merged
merged 49 commits into from
Jun 30, 2024

Conversation

JisanAR03
Copy link
Contributor

@JisanAR03 JisanAR03 commented Jun 4, 2024

Title: Completion of Project-1 and Resolution of Subtasks #2124 - #2132

Overview:

This pull request aims to finalize the development of Project-1 and resolves the associated subtasks specified below. Additionally, it introduces the necessary steps for server-side implementation, including running a management command and configuring the environment variables.


Api Endpoint :
path("api/ask/", question_answer_view, name="ask"),
so for call that api we have to use "{domain}/api/ask/"


Details:

Project-1: AI Chatbot for Whole Website from #2072

Subtasks Completed:

Server-Side Implementation:

To ensure the chatbot functions correctly on the server, the following steps are required:

  1. Run Management Command: Execute the command python manage.py update_faiss to update the FAISS index.
  2. Environment Configuration: Ensure the OPENAI_API_KEY is set in the .env file.

Conclusion:

This pull request completes the development and integration of the AI chatbot for the website, addressing all specified subtasks. The implementation has been thoroughly tested and refined to meet project requirements. Kindly review and merge this pull request to integrate the final changes into the project.


Thank you for your attention to this matter. Please let me know if there are any questions or further clarifications needed.

screenshot from implementation :
image

@AtmegaBuzz
Copy link
Collaborator

AtmegaBuzz commented Jun 4, 2024

If you are taking the api from env then can you also add gpt api key variable in .env.example. Also it would be great if you can add a check on api credits exhausted and prompt Bot is down

@JisanAR03
Copy link
Contributor Author

If you are taking the api from env then can you also add gpt api key variable in .env.example. Also it would be great if you can add a check on api credits exhausted and prompt Bot is down

Thank you sir, for this suggestion . I will recheck the error handling part ,when the api credit are exhausted

Copy link
Contributor

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

Please look at the following suggestion when you get a chance:

website/bot.py Outdated Show resolved Hide resolved
website/bot.py Outdated Show resolved Hide resolved
website/bot.py Outdated Show resolved Hide resolved
website/bot.py Outdated Show resolved Hide resolved
website/management/commands/update_faiss.py Outdated Show resolved Hide resolved
website/views.py Outdated Show resolved Hide resolved
website/views.py Outdated Show resolved Hide resolved
@JisanAR03
Copy link
Contributor Author

Please look at the following suggestion when you get a chance:

ok sure I will check those out

@JisanAR03
Copy link
Contributor Author

@arkid15r sir, updated the suggestion

@JisanAR03
Copy link
Contributor Author

If you are taking the api from env then can you also add gpt api key variable in .env.example. Also it would be great if you can add a check on api credits exhausted and prompt Bot is down

sir now update with error handling part too

Copy link
Contributor

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

@JisanAR03 very cool path refactoring!

I think it'd be great to narrow down exceptions in the cases like this except Exception as e: -- it's too broad. You can also omit e as it's unused.

blt/urls.py Outdated Show resolved Hide resolved
website/bot.py Outdated Show resolved Hide resolved
website/processed_files.txt Outdated Show resolved Hide resolved
website/bot.py Outdated Show resolved Hide resolved
website/bot.py Outdated Show resolved Hide resolved
website/management/commands/update_faiss.py Outdated Show resolved Hide resolved
@JisanAR03
Copy link
Contributor Author

@arkid15r sir , can you please review the latest commit

website/bot.py Outdated Show resolved Hide resolved
website/bot.py Outdated Show resolved Hide resolved
website/bot.py Outdated Show resolved Hide resolved
website/management/commands/update_faiss.py Outdated Show resolved Hide resolved
website/management/commands/update_faiss.py Outdated Show resolved Hide resolved
website/management/commands/update_faiss.py Outdated Show resolved Hide resolved
website/views.py Outdated Show resolved Hide resolved
website/views.py Outdated Show resolved Hide resolved
website/views.py Outdated Show resolved Hide resolved
website/views.py Outdated Show resolved Hide resolved
@DonnieBLT
Copy link
Collaborator

I was thinking it would also be good for us to send these messages to Slack to analyze them for accuracy

@JisanAR03 JisanAR03 closed this Jun 22, 2024
@JisanAR03 JisanAR03 reopened this Jun 22, 2024
@JisanAR03
Copy link
Contributor Author

@arkid15r sir, updated the code in new commit . can you please review ?

@JisanAR03 JisanAR03 requested a review from arkid15r June 22, 2024 09:23
Copy link
Contributor

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

A couple of minor suggestions.

Other than that -- looks good!

website/management/commands/update_faiss.py Outdated Show resolved Hide resolved
website/bot.py Outdated Show resolved Hide resolved
@JisanAR03 JisanAR03 requested a review from arkid15r June 29, 2024 07:34
@JisanAR03 JisanAR03 mentioned this pull request Jun 29, 2024
46 tasks
arkid15r
arkid15r previously approved these changes Jun 29, 2024
Copy link
Contributor

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

LGTM

@JisanAR03
Copy link
Contributor Author

@arkid15r sir, there was a conflict with another PR , that's why I make another commit 🙂

@arkid15r
Copy link
Contributor

@arkid15r sir, there was a conflict with another PR , that's why I make another commit 🙂

Yeah, that's fine. Thanks for fixing that!

@DonnieBLT DonnieBLT merged commit 733a330 into OWASP-BLT:main Jun 30, 2024
8 checks passed
@JisanAR03
Copy link
Contributor Author

@DonnieBLT thank you so much

@Uttkarsh-raj
Copy link
Contributor

Great work @JisanAR03. I had the same project idea and i think this pr also resolves the following issues.

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.

5 participants