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: Review server app setup #4432

Merged
merged 8 commits into from
Dec 22, 2023

Conversation

frascuchon
Copy link
Member

@frascuchon frascuchon commented Dec 19, 2023

Description

In this PR the argilla.server.server module has been removed and all server application setup has been moved to the argilla.server.app module.

Also, all the app creation flow has been wrapped into the create_server_app function

Type of change

(Please delete options that are not relevant. Remember to title the PR according to the type of change)

  • New feature (non-breaking change which adds functionality)
  • Refactor (change restructuring the codebase without changing functionality)
  • Improvement (change adding some improvement to an existing functionality)

How Has This Been Tested

(Please describe the tests that you ran to verify your changes. And ideally, reference tests)

Running locally `python -m argilla server start``

Checklist

  • I added relevant documentation
  • I followed the style guidelines of this project
  • I did a self-review of my code
  • I made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I filled out the contributor form (see text above)
  • I have added relevant notes to the CHANGELOG.md file (See https://keepachangelog.com/)

@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. area: server Indicates that an issue or pull request is related to the server language: python Pull requests or issues that update Python code status: invalid Indicates that an issue or pull request is no longer relevant team: backend Indicates that the issue or pull request is owned by the backend team type: refactor Indicates internal refactoring of the code-base labels Dec 19, 2023
@frascuchon frascuchon changed the title refactor: Create function to build and setup the argilla server refactor: Review server app setup Dec 19, 2023
@frascuchon frascuchon self-assigned this Dec 19, 2023
Copy link

The URL of the deployed environment for this PR is https://argilla-quickstart-pr-4432-ki24f765kq-no.a.run.app

Copy link

codecov bot commented Dec 19, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6630d7b) 90.13% compared to head (a1cc818) 91.32%.
Report is 552 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4432      +/-   ##
===========================================
+ Coverage    90.13%   91.32%   +1.18%     
===========================================
  Files          233      334     +101     
  Lines        12493    19315    +6822     
===========================================
+ Hits         11261    17640    +6379     
- Misses        1232     1675     +443     
Flag Coverage Δ
pytest ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

max_time=60,
on_backoff=_on_backoff,
)
def _setup_elasticsearch():
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should have a similar a similar function for Redis that do a ping to Redis (not here in your PR, I mean in my PR #4427) so we now early if Redis is not available, showing an error message.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 20, 2023
@frascuchon frascuchon added this to the v1.22.0 milestone Dec 22, 2023
@frascuchon frascuchon merged commit 49a2600 into develop Dec 22, 2023
20 checks passed
@frascuchon frascuchon deleted the refactor/create-server-app-function branch December 22, 2023 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: server Indicates that an issue or pull request is related to the server language: python Pull requests or issues that update Python code lgtm This PR has been approved by a maintainer size:XL This PR changes 500-999 lines, ignoring generated files. status: invalid Indicates that an issue or pull request is no longer relevant team: backend Indicates that the issue or pull request is owned by the backend team type: refactor Indicates internal refactoring of the code-base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants