Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

fix: Using settings.base_url to setup base url path #14

Merged
merged 8 commits into from
Feb 6, 2024

Conversation

frascuchon
Copy link
Member

Description

This PR introduces changes to allow configuring a base path using ARGILLA_BASE_URL without requiring an external proxy.

If you want to run argilla behind a proxy without changing base URL (which means defining a pathRewrite rule in your proxy), you should use the UVICORN_ROOT_PATH env variable. For more info visit https://fastapi.tiangolo.com/advanced/behind-a-proxy/#behind-a-proxy and https://www.uvicorn.org/settings/

Related PR: argilla-io/argilla#3543
Closes argilla-io/argilla#4568

Type of change

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

  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

How Has This Been Tested

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

  • Test A
  • Test B

Checklist

  • I followed the style guidelines of this project
  • I did a self-review of my code
  • 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/)

@frascuchon frascuchon changed the title fix: Using settings.base_url to setup base url path fix: Using settings.base_url to setup base url path Feb 5, 2024
Copy link

codecov bot commented Feb 5, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (6dff1c5) 90.13% compared to head (bd2791a) 90.14%.
Report is 3 commits behind head on main.

Files Patch % Lines
src/argilla_server/pydantic_v1/__init__.py 60.00% 2 Missing ⚠️
src/argilla_server/__init__.py 80.00% 1 Missing ⚠️
src/argilla_server/static_rewrite.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main      #14   +/-   ##
=======================================
  Coverage   90.13%   90.14%           
=======================================
  Files         187      187           
  Lines        9178     9190   +12     
=======================================
+ Hits         8273     8284   +11     
- Misses        905      906    +1     

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

@frascuchon frascuchon merged commit b37337e into main Feb 6, 2024
12 of 13 checks passed
@frascuchon frascuchon deleted the bugfixes/allow-configure-base-path-url branch February 6, 2024 08:08
frascuchon added a commit that referenced this pull request Feb 6, 2024
…ARGILLA_BASE_URL enabled

This fix is related to work developed in #14

This change is still compatible with a proxied configuration
frascuchon added a commit that referenced this pull request Feb 7, 2024
…e `ARGILLA_BASE_URL` enabled (#19)

This fix is related to work developed in #14

This change is still compatible with a proxied configuration. To make it
work using an nginx proxy configuration as defined
[here](https://github.com/argilla-io/argilla/blob/develop/docker/nginx/nginx.conf),
you should adapt the target server to adding the configured base URL:
```conf
events {}

http {
  server {
      listen 80;

      location /argilla/ {
          proxy_pass http://argilla:6900/argilla; # <- this line adds the `/argilla` path
          proxy_set_header Host $host;
          proxy_set_header X-Real-IP $remote_addr;
          proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
          proxy_set_header X-Forwarded-Proto $scheme;
      }
  }
}
```
frascuchon added a commit that referenced this pull request Feb 8, 2024
Otherwise, the SPA flow when loading the application from a shared URL

This error was introduced in this
[PR](#14)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG-python/deployment] ARGILLA_BASE_URL variable is working properly
2 participants