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

Server 4401 ingest space feedback #4468

Merged
merged 15 commits into from
Sep 3, 2024

Conversation

valeksiev
Copy link
Contributor

Describe the background of your pull request

Fixes: #4401

add calculated field status on VC
add VC updated subscription
add event handler trough CQRS for ingest space result
@valeksiev valeksiev requested review from techsmyth and hero101 August 26, 2024 15:19
Copy link
Member

@techsmyth techsmyth left a comment

Choose a reason for hiding this comment

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

Looks good!

Main concern is the linkage back from AI Server to VC, to be chatted through...

define handlers for incomming events only
add more logs
…io/server into server-4401-ingest-space-feedback
change bodyOfKnowledgeLastUpdated to be Date | null instead of undefined
@valeksiev
Copy link
Contributor Author

@techsmyth @hero101 We all dislike the dependency of the AiServer on the the VC repository (or some other part of the collaboration server), but I can't think of a way around it. This will need reworking when we split out the AiServer anyway.

Ignoring this I think I've addressed all comments and the implementation is complete.

@valeksiev valeksiev requested review from hero101 and techsmyth August 28, 2024 14:21
techsmyth
techsmyth previously approved these changes Aug 28, 2024
Copy link
Member

@techsmyth techsmyth left a comment

Choose a reason for hiding this comment

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

Understood re linkage back from AI server to collaboration server. To be addressed later.

@ccanos
Copy link
Contributor

ccanos commented Aug 30, 2024

Tested locally and it worked fine

  • Latest server-4401-ingest-space-feedback on both client and server.
  • Added AZURE_OPENAI_API_KEY=... to .env.docker
  • Modified AUTH_ADMIN_EMAIL and AUTH_ADMIN_PASSWORD in .env.docker - careful with special characters, you'll need to escape the $ sign, and maybe put the password in between double quotes.
  • Delete Exchange event-bus in RabbitMQ if the type is not direct. If the type is fanout get in, delete the exchange and restart services
  • Set to null all the bodyOfKnowledgeLastUpdated cells in the table ai_persona_service - careful with MySQLWorkbench, if safe updates is enabled you need to include an id column in all the WHERE clauses.
  • Navigate to http://localhost:3000/vc/<yourVC>/settings/settings
  • See that the status says Initializing:
    image
  • Open browser console and look for the graphql WebSocket
    image
  • Click on the button Update Knowledge
  • See the new message coming that updates the status
    image
  • See that in MySQL table ai_persona_service the VC's bodyOfKnowledgeLastUpdated column has been updated
  • I don't see any errors in the server or in the services logs

@ccanos
Copy link
Contributor

ccanos commented Aug 30, 2024

@valeksiev One note I just noticed, maybe it's okay, maybe you can't/don't want to do anything about it,
but the dates normally are stored in the database in UTC, and for the column bodyOfKnowledgeLastUpdated I have noticed it is storing current date as is. Or at least MySQLWorkbench is translating it to my timezone, not sure.
image

@valentinyanakiev valentinyanakiev merged commit 3d05252 into develop Sep 3, 2024
2 checks passed
@valentinyanakiev valentinyanakiev deleted the server-4401-ingest-space-feedback branch September 3, 2024 07:42
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.

Ingest service feedback on finished ingestion
5 participants