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

feat(api): add owner to corpus and auth to API. oauth2 provider start bearer token. frame CLI #7

Merged
merged 20 commits into from
Nov 2, 2024

Conversation

skyl
Copy link
Owner

@skyl skyl commented Oct 27, 2024

PR Type

Enhancement, Tests


Description

  • Implemented a comprehensive Corpora API client with CRUD operations for corpora and files.
  • Developed an ApiClient for handling HTTP requests and responses, including serialization and authentication.
  • Created configuration management for API client settings, including logging and SSL support.
  • Added a REST client for making HTTP requests with error handling.
  • Defined custom exceptions for API error handling and response validation.
  • Added async test cases for API endpoints with OAuth2 authentication.
  • Developed models for corpus and file response schemas with serialization methods.
  • Set up a Typer CLI application for managing corpora with integrated authentication.

PRDescriptionHeader.CHANGES_WALKTHROUGH

Relevant files
Enhancement
8 files
corpora_api.py
Implement Corpora API client with CRUD operations               

py/gen/corpora_client/corpora_client/api/corpora_api.py

  • Added CorporaApi class with methods for creating and retrieving
    corpora and files.
  • Implemented serialization and deserialization for API requests and
    responses.
  • Added authentication settings using Bearer tokens.
  • +1256/-0
    api_client.py
    Add ApiClient for handling HTTP requests and responses     

    py/gen/corpora_client/corpora_client/api_client.py

  • Introduced ApiClient class for handling API requests.
  • Added methods for serializing parameters and deserializing responses.
  • Implemented authentication and header management.
  • +755/-0 
    configuration.py
    Create Configuration class for API client settings             

    py/gen/corpora_client/corpora_client/configuration.py

  • Created Configuration class for managing API client settings.
  • Added support for authentication and server configuration.
  • Implemented logging and SSL settings.
  • +476/-0 
    rest.py
    Develop REST client for HTTP request handling                       

    py/gen/corpora_client/corpora_client/rest.py

  • Developed RESTClientObject for making HTTP requests.
  • Implemented support for different HTTP methods and content types.
  • Added error handling for SSL and connection issues.
  • +246/-0 
    exceptions.py
    Define custom exceptions for API error handling                   

    py/gen/corpora_client/corpora_client/exceptions.py

  • Defined custom exceptions for API errors.
  • Implemented exception handling for HTTP response errors.
  • Added utility functions for rendering error paths.
  • +199/-0 
    file_response_schema.py
    Create FileResponseSchema model for file data representation

    py/gen/corpora_client/corpora_client/models/file_response_schema.py

  • Created FileResponseSchema model for file response data.
  • Implemented methods for JSON and dictionary serialization.
  • +116/-0 
    corpus_response_schema.py
    Develop CorpusResponseSchema model for corpus data representation

    py/gen/corpora_client/corpora_client/models/corpus_response_schema.py

  • Developed CorpusResponseSchema model for corpus response data.
  • Added serialization and deserialization methods.
  • +107/-0 
    main.py
    Set up Typer CLI for corpora management with authentication

    py/packages/corpora_cli/main.py

  • Set up Typer CLI application for managing corpora.
  • Integrated authentication and API client initialization.
  • Registered corpus and file command modules.
  • +34/-0   
    Tests
    2 files
    test_api.py
    Add async test cases for API endpoints with OAuth2 authentication

    py/packages/corpora/test_api.py

  • Added async test cases for corpus and file creation and retrieval.
  • Implemented OAuth2 authentication for API tests.
  • Included tests for error scenarios like not found resources.
  • +89/-21 
    test_config.py
    Add tests for CLI configuration loading and validation     

    py/packages/corpora_cli/test_config.py

  • Added tests for configuration loading and environment variable
    substitution.
  • Implemented tests for handling missing and invalid configuration
    files.
  • +107/-0 
    Additional files (token-limit)
    60 files
    auth.py
    ...                                                                                                           

    py/packages/corpora_cli/auth.py

    ...

    +106/-0 
    test_file_response_schema.py
    ...                                                                                                           

    py/gen/corpora_client/test/test_file_response_schema.py

    ...

    +76/-0   
    corpus_schema.py
    ...                                                                                                           

    py/gen/corpora_client/corpora_client/models/corpus_schema.py

    ...

    +89/-0   
    file_schema.py
    ...                                                                                                           

    py/gen/corpora_client/corpora_client/models/file_schema.py

    ...

    +91/-0   
    test_auth.py
    ...                                                                                                           

    py/packages/corpora_cli/test_auth.py

    ...

    +76/-0   
    test_corpus_response_schema.py
    ...                                                                                                           

    py/gen/corpora_client/test/test_corpus_response_schema.py

    ...

    +61/-0   
    config.py
    ...                                                                                                           

    py/packages/corpora_cli/config.py

    ...

    +46/-0   
    test_corpora_api.py
    ...                                                                                                           

    py/gen/corpora_client/test/test_corpora_api.py

    ...

    +66/-0   
    setup.py
    ...                                                                                                           

    py/gen/corpora_client/setup.py

    ...

    +49/-0   
    test_file_schema.py
    ...                                                                                                           

    py/gen/corpora_client/test/test_file_schema.py

    ...

    +58/-0   
    test_corpus_schema.py
    ...                                                                                                           

    py/gen/corpora_client/test/test_corpus_schema.py

    ...

    +55/-0   
    api.py
    ...                                                                                                           

    py/packages/corpora/api.py

    ...

    +21/-5   
    __init__.py
    ...                                                                                                           

    py/gen/corpora_client/corpora_client/init.py

    ...

    +37/-0   
    settings.py
    ...                                                                                                           

    py/packages/corpora_proj/settings.py

    ...

    +6/-4     
    auth.py
    ...                                                                                                           

    py/packages/corpora/auth.py

    ...

    +29/-0   
    0003_corpus_owner.py
    ...                                                                                                           

    py/packages/corpora/migrations/0003_corpus_owner.py

    ...

    +28/-0   
    api_response.py
    ...                                                                                                           

    py/gen/corpora_client/corpora_client/api_response.py

    ...

    +20/-0   
    __init__.py
    ...                                                                                                           

    py/gen/corpora_client/corpora_client/models/init.py

    ...

    +20/-0   
    models.py
    ...                                                                                                           

    py/packages/corpora/models.py

    ...

    +9/-0     
    urls.py
    ...                                                                                                           

    py/packages/corpora_proj/urls.py

    ...

    +20/-0   
    file.py
    ...                                                                                                           

    py/packages/corpora_cli/commands/file.py

    ...

    +17/-0   
    corpus.py
    ...                                                                                                           

    py/packages/corpora_cli/commands/corpus.py

    ...

    +18/-0   
    asgi.py
    ...                                                                                                           

    py/packages/corpora_proj/asgi.py

    ...

    +2/-2     
    wsgi.py
    ...                                                                                                           

    py/packages/corpora_proj/wsgi.py

    ...

    +2/-2     
    manage.py
    ...                                                                                                           

    py/packages/corpora_proj/manage.py

    ...

    +1/-1     
    __init__.py
    ...                                                                                                           

    py/gen/corpora_client/corpora_client/api/init.py

    ...

    +4/-0     
    git_push.sh
    ...                                                                                                           

    py/gen/corpora_client/git_push.sh

    ...

    +57/-0   
    setup.sh
    ...                                                                                                           

    .devcontainer/setup.sh

    ...

    +3/-2     
    genall.sh
    ...                                                                                                           

    py/genall.sh

    ...

    +12/-0   
    CorporaApi.md
    ...                                                                                                           

    py/gen/corpora_client/docs/CorporaApi.md

    ...

    +401/-0 
    README.md
    ...                                                                                                           

    py/gen/corpora_client/README.md

    ...

    +127/-0 
    pyproject.toml
    ...                                                                                                           

    py/gen/corpora_client/pyproject.toml

    ...

    +72/-0   
    .corpora.yaml
    ...                                                                                                           

    .corpora.yaml

    ...

    +51/-0   
    README.md
    ...                                                                                                           

    py/packages/corpora/README.md

    ...

    +14/-0   
    Dockerfile
    ...                                                                                                           

    .devcontainer/Dockerfile

    ...

    +10/-17 
    TODO.md
    ...                                                                                                           

    TODO.md

    ...

    +12/-5   
    FileResponseSchema.md
    ...                                                                                                           

    py/gen/corpora_client/docs/FileResponseSchema.md

    ...

    +35/-0   
    CorpusResponseSchema.md
    ...                                                                                                           

    py/gen/corpora_client/docs/CorpusResponseSchema.md

    ...

    +33/-0   
    FILES
    ...                                                                                                           

    py/gen/corpora_client/.openapi-generator/FILES

    ...

    +38/-0   
    .gitlab-ci.yml
    ...                                                                                                           

    py/gen/corpora_client/.gitlab-ci.yml

    ...

    +31/-0   
    .openapi-generator-ignore
    ...                                                                                                           

    py/gen/corpora_client/.openapi-generator-ignore

    ...

    +23/-0   
    python.yml
    ...                                                                                                           

    py/gen/corpora_client/.github/workflows/python.yml

    ...

    +31/-0   
    FileSchema.md
    ...                                                                                                           

    py/gen/corpora_client/docs/FileSchema.md

    ...

    +31/-0   
    CorpusSchema.md
    ...                                                                                                           

    py/gen/corpora_client/docs/CorpusSchema.md

    ...

    +30/-0   
    .travis.yml
    ...                                                                                                           

    py/gen/corpora_client/.travis.yml

    ...

    +17/-0   
    README.md
    ...                                                                                                           

    py/packages/README.md

    ...

    +1/-1     
    ci-python.yml
    ...                                                                                                           

    .github/workflows/ci-python.yml

    ...

    +3/-0     
    pr-agent.yml
    ...                                                                                                           

    .github/workflows/pr-agent.yml

    ...

    +1/-0     
    test-requirements.txt
    ...                                                                                                           

    py/gen/corpora_client/test-requirements.txt

    ...

    +6/-0     
    requirements.txt
    ...                                                                                                           

    py/requirements.txt

    ...

    +5/-0     
    tox.ini
    ...                                                                                                           

    py/gen/corpora_client/tox.ini

    ...

    +9/-0     
    requirements.txt
    ...                                                                                                           

    py/packages/corpora_cli/requirements.txt

    ...

    +5/-0     
    requirements.txt
    ...                                                                                                           

    py/gen/corpora_client/requirements.txt

    ...

    +4/-0     
    openapitools.json
    ...                                                                                                           

    py/openapitools.json

    ...

    +7/-0     
    requirements.txt
    ...                                                                                                           

    py/packages/corpora/requirements.txt

    ...

    +1/-0     
    README.md
    ...                                                                                                           

    py/packages/corpora_proj/README.md

    ...

    +7/-1     
    pytest.ini
    ...                                                                                                           

    py/pytest.ini

    ...

    +1/-1     
    setup.cfg
    ...                                                                                                           

    py/gen/corpora_client/setup.cfg

    ...

    +2/-0     
    VERSION
    ...                                                                                                           

    py/gen/corpora_client/.openapi-generator/VERSION

    ...

    +1/-0     
    .gitattributes
    ...                                                                                                           

    .gitattributes

    ...

    +2/-0     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    github-actions bot commented Oct 27, 2024

    PR Reviewer Guide 🔍

    (Review updated until commit ed68067)

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Code Smell
    The file corpora_api.py is auto-generated and contains a large amount of boilerplate code. Ensure that the generated code aligns with the project's coding standards and that any necessary customizations are correctly implemented.

    Code Smell
    The file api_client.py is auto-generated and contains a large amount of boilerplate code. Ensure that the generated code aligns with the project's coding standards and that any necessary customizations are correctly implemented.

    Test Coverage
    The test cases in test_api.py have been updated to include authentication headers. Ensure that these tests cover all relevant scenarios and edge cases for the new authentication logic.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Validate that request.user is authenticated before using it to prevent errors

    Ensure that the request.user is always set to a valid user object before accessing
    it to prevent potential attribute errors.

    py/packages/corpora/api.py [15-17]

    +if request.user is None:
    +    # Handle the case where user is not authenticated
     corpus = await Corpus.objects.acreate(
         name=payload.name, url=payload.url, owner=request.user
     )
    Suggestion importance[1-10]: 8

    Why: This suggestion addresses a potential bug by ensuring that request.user is authenticated before being used, which can prevent attribute errors and improve the robustness of the code.

    8
    Maintainability
    Use a setup method to create common test data for improved maintainability

    Consider using a setup method to create common test data to reduce redundancy and
    improve test maintainability.

    py/packages/corpora/test_api.py [61-62]

    -user, headers = await create_user_and_token()
    -corpus = await create_corpus("Retrieve Corpus", "https://example.com", user)
    +user, headers, corpus = await setup_test_data()
    Suggestion importance[1-10]: 7

    Why: This suggestion improves test maintainability by reducing redundancy, making the tests easier to manage and update. It is a valuable enhancement for code organization.

    7
    Security
    Replace hardcoded access tokens with securely generated tokens in tests

    Use a more secure method for generating access tokens in tests to avoid potential
    security issues.

    py/packages/corpora/test_api.py [31]

    -token="testaccesstoken",
    +token=generate_secure_token(),
    Suggestion importance[1-10]: 6

    Why: While using securely generated tokens is a good practice, the suggestion is less critical in the context of tests. However, it does enhance security practices and test reliability.

    6

    @skyl skyl changed the title feat(api): add owner to corpus and auth to API. oauth2 provider start bearer token feat(api): add owner to corpus and auth to API. oauth2 provider start bearer token. frame CLI Oct 28, 2024
    @skyl
    Copy link
    Owner Author

    skyl commented Oct 30, 2024

    /review

    @skyl
    Copy link
    Owner Author

    skyl commented Oct 30, 2024

    /describe

    Copy link

    Persistent review updated to latest commit ed68067

    Copy link

    PR Description updated to latest commit (ed68067)

    @skyl
    Copy link
    Owner Author

    skyl commented Oct 30, 2024

    /ask "This is an early pull request to a new, ambitious repo. The scope has grown too large for a single change request. But, the changes will go in now. Can you please summarize the TODO items that have been left in various files of this PR?"

    Copy link

    Ask

    this is an early pull request to a new, ambitious repo. the scope has grown too large for a single change request. but, the changes will go in now. can you please summarize the todo items that have been left in various files of this pr?

    Answer:

    The provided PR diff does not explicitly show any TODO comments within the code. However, given the nature of the changes and the description provided, here are some potential areas where TODO items might be relevant:

    1. Error Handling and Validation:

      • Ensure comprehensive error handling and validation for all API endpoints. This includes handling edge cases and providing meaningful error messages.
    2. Authentication and Security:

      • Review and enhance the OAuth2 authentication implementation to ensure it meets security best practices.
      • Consider implementing additional security measures, such as rate limiting or IP whitelisting.
    3. Documentation:

      • Complete the documentation for the API, including usage examples and detailed descriptions of each endpoint.
      • Ensure that the CLI tool is documented, with examples of common use cases.
    4. Testing:

      • Expand the test coverage to include more edge cases and potential failure scenarios.
      • Consider adding performance tests to ensure the API can handle expected loads.
    5. Code Optimization:

      • Review the code for potential optimizations, especially in the API client and serialization/deserialization logic.
      • Ensure that the code adheres to PEP 8 standards and refactor any sections that could be improved for readability or efficiency.
    6. Feature Completeness:

      • Verify that all intended features for this phase are implemented and functioning as expected.
      • Plan for any additional features that were deferred due to the scope of this PR.
    7. Configuration Management:

      • Ensure that configuration management is robust and allows for easy changes to API settings, such as base URLs and authentication credentials.
    8. Deployment and CI/CD:

      • Set up continuous integration and deployment pipelines to automate testing and deployment processes.
      • Ensure that the deployment process is documented and repeatable.

    These areas are inferred based on typical development practices and the scope described in the PR. It would be beneficial to review the actual codebase for any specific TODO comments or areas that require further attention.

    @skyl
    Copy link
    Owner Author

    skyl commented Oct 30, 2024

    The provided PR diff does not explicitly show any TODO comments within the code. However, given the nature of the changes and the description provided, here are some potential areas where TODO items might be relevant:

    I'm guessing that somewhere in the pipeline the token limit was hit. Maybe we should find a different way than checking in the generated code.

    @skyl skyl merged commit 6259461 into main Nov 2, 2024
    1 check passed
    @skyl skyl deleted the auth branch November 2, 2024 01:26
    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.

    1 participant