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

Fix MilvusMemory connecting to Zilliz #3278

Merged
merged 4 commits into from
Apr 27, 2023
Merged

Fix MilvusMemory connecting to Zilliz #3278

merged 4 commits into from
Apr 27, 2023

Conversation

chyezh
Copy link
Contributor

@chyezh chyezh commented Apr 26, 2023

Background

A new bug was introduced by #2127

Changes

fix new bug in MilvusMemory construction.

Documentation

Test Plan

  • pass manually test

PR Quality Checklist

  • My pull request is atomic and focuses on a single change.
  • I have thoroughly tested my changes with multiple different prompts.
  • I have considered potential risks and mitigations for my changes.
  • I have documented my changes clearly and comprehensively.
  • I have not snuck in any "extra" small tweaks changes

@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Patch coverage has no change and project coverage change: -7.98 ⚠️

Comparison is base (02f546d) 51.22% compared to head (c4c3e73) 43.25%.

❗ Current head c4c3e73 differs from pull request most recent head d61f31d. Consider uploading reports for the commit d61f31d to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3278      +/-   ##
==========================================
- Coverage   51.22%   43.25%   -7.98%     
==========================================
  Files          65       65              
  Lines        2975     2973       -2     
  Branches      502      502              
==========================================
- Hits         1524     1286     -238     
- Misses       1331     1612     +281     
+ Partials      120       75      -45     
Impacted Files Coverage Δ
autogpt/memory/milvus.py 3.38% <0.00%> (ø)

... and 18 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@chyezh
Copy link
Contributor Author

chyezh commented Apr 26, 2023

@Pwuts Hi, It's the bugfix commit for the fatal bug introduced by #2127.
The bug is already discussed in another PR #3084 (comment)
But the merging of #3084 is blocked because of conflict of test with #1725. So I will submit the related test of Milvus after #1725 is merged, and produce this PR for the bugfix separately.
I think that this fatal bug will be fixed as soon as possible.

@chyezh
Copy link
Contributor Author

chyezh commented Apr 27, 2023

Hi, @Pwuts, Could you please review and fix this fatal error? :)
User using Zilliz Cloud will encounter this issue.
There are only 7 lines of code changes here, but it is very important to us.

@vercel
Copy link

vercel bot commented Apr 27, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 27, 2023 1:56am

@vercel vercel bot temporarily deployed to Preview April 27, 2023 00:55 Inactive
@Pwuts Pwuts self-assigned this Apr 27, 2023
@Pwuts Pwuts changed the title fix connection bug for zilliz uri on milvus Fix MilvusMemory connecting to Zilliz Apr 27, 2023
@Pwuts Pwuts added bug Something isn't working function: memory labels Apr 27, 2023
@vercel vercel bot temporarily deployed to Preview April 27, 2023 01:56 Inactive
@waynehamadi waynehamadi merged commit 65b6c27 into Significant-Gravitas:master Apr 27, 2023
@Pwuts Pwuts added this to the v0.3.0 release milestone Apr 27, 2023
skarvsladd

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working function: memory size/m
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants