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

Message about Pinecone initializing #1194

Merged
merged 4 commits into from
May 1, 2023
Merged

Message about Pinecone initializing #1194

merged 4 commits into from
May 1, 2023

Conversation

Ashutoshkataria19
Copy link
Contributor

@Ashutoshkataria19 Ashutoshkataria19 commented Apr 13, 2023

Added a message: "Connecting Pinecone. This may take some time..."
close #987

Background

If the Pinecone index setup takes a noticeable amount of time, the console just stops. It is necessary to notify the user that the index is being configured now and this may take some time.

Changes

Added a message: "Connecting Pinecone. This may take some time..."

Documentation

Test Plan

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

@Ashutoshkataria19 Ashutoshkataria19 changed the title Message about Pinecone initializing #987 Message about Pinecone initializing Apr 13, 2023
Copy link
Contributor

@nponeccop nponeccop left a comment

Choose a reason for hiding this comment

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

Look what we use for messages. You cannot just print.

@Ashutoshkataria19
Copy link
Contributor Author

Ashutoshkataria19 commented Apr 14, 2023

please I need some help as this is my first pull request
I read logger.py and I can think of these three approaches
please let me know if I am wrong

1st approach
I can use logger.debug("Connecting Pinecone. This may take some time...")

but this message is of level = logging.info

2nd approach
first I should define something like
def info( self, message, title='', title_color='', ): self._log(title, title_color, message, logging.INFO)
in logger.py and then use
logger.info("Connecting Pinecone. This may take some time...") in pinecone.py

3rd approach
or should I directly go with
logger._log('', '', "Connecting Pinecone. This may take some time...", logging.INFO)

@nponeccop
Copy link
Contributor

@p-i- @richbeales Could you assist him?

@Ashutoshkataria19
Copy link
Contributor Author

Waiting for some hint.
At least tell me if I am thinking in right direction.

@nponeccop
Copy link
Contributor

Maybe @BillSchumacher ?

@github-actions github-actions bot added conflicts Automatically applied to PRs with merge conflicts labels Apr 17, 2023
@github-actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

3 similar comments
@github-actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

Copy link
Member

@ntindle ntindle left a comment

Choose a reason for hiding this comment

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

use the logger as intended

scripts/memory/pinecone.py Outdated Show resolved Hide resolved
@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Apr 18, 2023
@github-actions
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@ntindle
Copy link
Member

ntindle commented Apr 18, 2023

You accidentally deleted the file. Revert that commit please. You can do it from GitHub desktop UI or switch to your branch and run

git revert 70dc1af4c8322be4033df2920a85c8e25688f3ae

and then commit and push that

Copy link
Member

@ntindle ntindle left a comment

Choose a reason for hiding this comment

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

Fix file deletion

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Apr 18, 2023
@github-actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@Ashutoshkataria19
Copy link
Contributor Author

You accidentally deleted the file. Revert that commit please. You can do it from GitHub desktop UI or switch to your branch and run

git revert 70dc1af4c8322be4033df2920a85c8e25688f3ae

and then commit and push that

thanks for pointing out

@Ashutoshkataria19
Copy link
Contributor Author

I apologize to everyone for any problems or difficulties I may have caused and I appreciate your time and patience.

ntindle
ntindle previously approved these changes Apr 18, 2023
@k-boikov
Copy link
Contributor

@Ashutoshkataria19 the PRs is outdated, the file is now autogpt/memory/pinecone.py not scripts/memory/pinecone.py please rebase.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Apr 18, 2023
@github-actions
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@Ashutoshkataria19
Copy link
Contributor Author

I attempted to rebase my branch, hoping that updating my fork would resolve the issues. However, during the process, all the commits were inadvertently deleted, and the pull request was closed. I have now reopened the pull request with the same changes that @ntindle had approved. Could you please review the pull request again?

nponeccop
nponeccop previously approved these changes Apr 19, 2023
ntindle
ntindle previously approved these changes Apr 23, 2023
@codecov
Copy link

codecov bot commented Apr 23, 2023

Codecov Report

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

Comparison is base (aedd288) 49.20% compared to head (979d147) 40.69%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1194      +/-   ##
==========================================
- Coverage   49.20%   40.69%   -8.51%     
==========================================
  Files          63       63              
  Lines        3014     3015       +1     
  Branches      496      496              
==========================================
- Hits         1483     1227     -256     
- Misses       1411     1716     +305     
+ Partials      120       72      -48     
Impacted Files Coverage Δ
autogpt/memory/pinecone.py 27.90% <0.00%> (-0.67%) ⬇️

... and 15 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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

@Vwing
Copy link
Contributor

Vwing commented May 1, 2023

@Ashutoshkataria19 This failed the lint check. Please run black . to fix the formatting issues.

@Vwing Vwing requested review from Vwing and removed request for Vwing May 1, 2023 00:53
richbeales
richbeales previously approved these changes May 1, 2023
@vercel
Copy link

vercel bot commented May 1, 2023

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

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
docs ⬜️ Ignored (Inspect) Visit Preview May 1, 2023 2:27pm

@vercel vercel bot temporarily deployed to Preview May 1, 2023 14:26 Inactive
@richbeales richbeales dismissed stale reviews from ntindle, nponeccop, and themself via 237599c May 1, 2023 14:27
@github-actions github-actions bot added size/s and removed size/xs labels May 1, 2023
@richbeales richbeales merged commit 9c56b1b into Significant-Gravitas:master May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Massage about Pinecone initializing
6 participants