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

Add object storage #213

Merged
merged 1 commit into from
Apr 16, 2023
Merged

Add object storage #213

merged 1 commit into from
Apr 16, 2023

Conversation

junjiejiangjjj
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Apr 15, 2023

Codecov Report

Merging #213 (fd4fa35) into dev (39faf3a) will decrease coverage by 1.52%.
The diff coverage is 80.00%.

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

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #213      +/-   ##
==========================================
- Coverage   92.89%   91.37%   -1.52%     
==========================================
  Files          42       47       +5     
  Lines        1365     1496     +131     
==========================================
+ Hits         1268     1367      +99     
- Misses         97      129      +32     
Impacted Files Coverage Δ
gptcache/manager/object_data/s3_storage.py 44.11% <44.11%> (ø)
gptcache/manager/object_data/manager.py 64.28% <64.28%> (ø)
gptcache/manager/object_data/base.py 73.33% <73.33%> (ø)
gptcache/manager/object_data/local_storage.py 85.71% <85.71%> (ø)
gptcache/adapter/adapter.py 94.33% <100.00%> (-0.11%) ⬇️
gptcache/adapter/openai.py 93.81% <100.00%> (+0.06%) ⬆️
gptcache/manager/__init__.py 100.00% <100.00%> (ø)
gptcache/manager/data_manager.py 88.61% <100.00%> (+1.82%) ⬆️
gptcache/manager/factory.py 100.00% <100.00%> (+8.33%) ⬆️
gptcache/manager/object_data/__init__.py 100.00% <100.00%> (ø)
... and 3 more

"question": cache_question,
"answer": cache_answer,
"question": ret.question,
"answer": ret.answer[0].answer,
Copy link
Collaborator

Choose a reason for hiding this comment

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

answers is more suitable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

},
extra_param=context.get("evaluation_func", None),
)
if rank_threshold <= rank:
cache_answers.append((rank, cache_answer))
cache_answers.append((rank, ret.answer[0].answer))
Copy link
Collaborator

Choose a reason for hiding this comment

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

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -180,10 +188,16 @@ def import_data(
normalize(embedding_data) for embedding_data in embedding_datas
]
for i, embedding_data in enumerate(embedding_datas):
if not isinstance(answers[i], str):
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe is a list, one question multiple answers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

self.import_data([question], [answer], [embedding_data])

def _process_answers(self, answers: Union[Answer, List[Answer]]):
Copy link
Collaborator

Choose a reason for hiding this comment

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

_process_answers -> _process_answer_data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

"""Local object storage
"""

def __init__(self, local_root):
Copy link
Collaborator

Choose a reason for hiding this comment

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

add param type, like: local_root: str

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

try:
os.remove(obj)
except Exception: # pylint: disable=broad-except
pass
Copy link
Collaborator

Choose a reason for hiding this comment

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

give some log

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

def get(name, **kwargs):
if name == "local":
from gptcache.manager.object_data.local_storage import LocalObjectStorage # pylint: disable=import-outside-toplevel
object_base = LocalObjectStorage(kwargs.get("path", "./"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

the default value ./local_obj may be more suitable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

elif name == "s3":
from gptcache.manager.object_data.s3_storage import S3Storage # pylint: disable=import-outside-toplevel
object_base = S3Storage(kwargs.get("path_prefix"), kwargs.get("bucket"),
kwargs.get("access_key"), kwargs.get("secret_key"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

need endpoint param

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

question: Any
answer: Union[Any, List[Any]]
answer: List[Answer]
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename it, answers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Signed-off-by: junjie.jiang <junjie.jiang@zilliz.com>
@SimFG
Copy link
Collaborator

SimFG commented Apr 16, 2023

/lgtm
/approve

@sre-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: junjiejiangjjj, SimFG

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot merged commit 211949a into zilliztech:dev Apr 16, 2023
7 checks passed
@junjiejiangjjj junjiejiangjjj deleted the dev_obj branch April 16, 2023 13:31
SimFG pushed a commit that referenced this pull request Apr 17, 2023
Signed-off-by: junjie.jiang <junjie.jiang@zilliz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants