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

refactor(API): Refactor datasets API #2439

Merged
merged 15 commits into from
Sep 18, 2024
Merged

Conversation

Valdanitooooo
Copy link
Contributor

@Valdanitooooo Valdanitooooo commented Sep 14, 2024

What problem does this PR solve?

discuss:#1102

Completed

  1. Integrate API Flask to generate Swagger API documentation, through http://ragflow_host:ragflow_port/v1/docs visit
  2. Refactored http_token_auth
class AuthUser:
    def __init__(self, tenant_id, token):
        self.id = tenant_id
        self.token = token

    def get_token(self):
        return self.token


@http_token_auth.verify_token
def verify_token(token: str) -> Union[AuthUser, None]:
    try:
        objs = APIToken.query(token=token)
        if objs:
            api_token = objs[0]
            user = AuthUser(api_token.tenant_id, api_token.token)
            return user
    except Exception as e:
        server_error_response(e)
    return None

# resources api
@manager.auth_required(http_token_auth)
def get_all_datasets(query_data):
	....
  1. Refactored the Datasets (Knowledgebase) API to extract the implementation logic into the api/apps/services directory
    image
  2. Python SDK, I only added get_all_datasets as an attempt, Just to verify that SDK API and Server API can use the same method.
from ragflow.ragflow import RAGFLow
ragflow = RAGFLow('<ACCESS_KEY>', 'http://127.0.0.1:9380')
ragflow.get_all_datasets()
  1. Request parameter validation, as an attempt, may not be necessary as this feature is already present at the data model layer. This is mainly easier to test the API in Swagger Docs service
class UpdateDatasetReq(Schema):
    kb_id = fields.String(required=True)
    name = fields.String(validate=validators.Length(min=1, max=128))
    description = fields.String(allow_none=True)
    permission = fields.String(validate=validators.OneOf(['me', 'team']))
    embd_id = fields.String(validate=validators.Length(min=1, max=128))
    language = fields.String(validate=validators.OneOf(['Chinese', 'English']))
    parser_id = fields.String(validate=validators.OneOf([parser_type.value for parser_type in ParserType]))
    parser_config = fields.Dict()
    avatar = fields.String()

TODO

  1. Simultaneously supporting multiple authentication methods, so that the Web API can use the same method as the Server API, but perhaps this feature is not important.
    I tried using this method, but it was not successful. It only allows token authentication when not logged in, but cannot skip token authentication when logged in 😢
def http_basic_auth_required(func):
    @wraps(func)
    def decorated_view(*args, **kwargs):
        if 'Authorization' in flask_request.headers:
            # If the request header contains a token, skip username and password verification
            return func(*args, **kwargs)
        if flask_request.method in EXEMPT_METHODS or current_app.config.get("LOGIN_DISABLED"):
            pass
        elif not current_user.is_authenticated:
            return current_app.login_manager.unauthorized()

        if callable(getattr(current_app, "ensure_sync", None)):
            return current_app.ensure_sync(func)(*args, **kwargs)
        return func(*args, **kwargs)

    return decorated_view
  1. Refactoring the SDK API using the same method as the Server API is feasible and constructive, but it still requires time
    I see some differences between the Web and SDK APIs, such as the key_mapping handling of the returned results. Until I figure it out, I cannot modify these codes to avoid causing more problems
    for kb in kbs:
        key_mapping = {
            "chunk_num": "chunk_count",
            "doc_num": "document_count",
            "parser_id": "parse_method",
            "embd_id": "embedding_model"
        }
        renamed_data = {}
        for key, value in kb.items():
            new_key = key_mapping.get(key, key)
            renamed_data[new_key] = value
        renamed_list.append(renamed_data)
    return get_json_result(data=renamed_list)

Type of change

  • Refactoring

@Valdanitooooo Valdanitooooo changed the title API: add retrieval api refactor(API): Refactor datasets API Sep 14, 2024
@JinHai-CN
Copy link
Contributor

Thank you for your PR. Would you please use English to write the source code comment?

@Valdanitooooo
Copy link
Contributor Author

Thank you for your PR. Would you please use English to write the source code comment?

Of course, I will

@KevinHuSh
Copy link
Collaborator

Thank you for your PR. Would you please use English to write the source code comment?

Of course, I will

Thanks a lot.
Could you move this PR to branch 'api'. It's more convinient for us to move forward.

@Valdanitooooo
Copy link
Contributor Author

Valdanitooooo commented Sep 18, 2024

move this PR to branch 'api'

👌

@Valdanitooooo Valdanitooooo changed the base branch from main to api September 18, 2024 06:49
@Valdanitooooo
Copy link
Contributor Author

@KevinHuSh Done.

@KevinHuSh KevinHuSh merged commit 5c77792 into infiniflow:api Sep 18, 2024
1 check passed
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.

3 participants