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 API versioning #107

Merged
merged 4 commits into from
Mar 31, 2024
Merged

Add API versioning #107

merged 4 commits into from
Mar 31, 2024

Conversation

delano
Copy link
Contributor

@delano delano commented Mar 31, 2024

This pull request adds API versioning to the project. The DEFAULT_VERSIONING_CLASS setting is set to rest_framework.versioning.URLPathVersioning, and the allowed versions are v1. The VERSION_PARAM is set to version.

@delano delano self-assigned this Mar 31, 2024
Copy link
Contributor

PR Review

⏱️ Estimated effort to review [1-5]

2, because the changes are straightforward and mostly involve adding versioning to the API paths and settings. However, careful attention is needed to ensure versioning is correctly implemented across different files and that no endpoints are missed.

🧪 Relevant tests

No

🔍 Possible issues

Possible Bug: The versioning added to the API might not be fully backward compatible if there are existing clients that do not expect the version in the URL. This could lead to broken integrations.

🔒 Security concerns

No

apps/api/afb/urls/__init__.py Show resolved Hide resolved
apps/api/afb/settings.py Show resolved Hide resolved
apps/api/afbcore/views/users.py Show resolved Hide resolved
apps/ui/pages/login/index.vue Show resolved Hide resolved
Copy link
Contributor

qodo-merge-pro bot commented Mar 31, 2024

PR Code Suggestions

CategorySuggestions                                                                                                                                                       
Enhancement
Add more API versions to the allowed versions list.

Consider adding more versions to "ALLOWED_VERSIONS" to support future versioning and
ensure backward compatibility.

apps/api/afb/settings.py [246]

-"ALLOWED_VERSIONS": ["v1"],
+"ALLOWED_VERSIONS": ["v1", "v2"],
 
Best practice
Use a specific path converter for API versioning in URLs.

It's recommended to use a more specific path converter than str:version for the version parameter in
the URL to enforce a consistent versioning format.

apps/api/afb/urls/init.py [24]

-path("api/<str:version>/", include(router.urls)),
+path("api/v<int:version>/", include(router.urls)),
 
Use environment variables for API base URL configuration.

Similar to the previous suggestion, consider using an environment variable for the API
base URL in the Nuxt configuration to facilitate changes in the future.

apps/ui/nuxt.config.ts [68]

-baseURL: '/api/v1',
+baseURL: `/api/${process.env.API_VERSION || 'v1'}`,
 
Maintainability
Remove unused method parameters.

Ensure that the version parameter is actually used within the current_user method or
consider removing it if unnecessary.

apps/api/afbcore/views/users.py [42]

-def current_user(self, request, version=None, *args, **kwargs):
+def current_user(self, request, *args, **kwargs):
 
Avoid hardcoding API version in the path.

Extract the API version into a configuration or environment variable to avoid hardcoding
it across different files.

apps/ui/pages/login/index.vue [81]

-const path = "/api/v1/passwordless/auth/email/";
+const apiVersion = process.env.API_VERSION || 'v1';
+const path = `/api/${apiVersion}/passwordless/auth/email/`;
 

✨ Improve tool usage guide:

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

  • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...
[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

Copy link
Contributor

qodo-merge-pro bot commented Mar 31, 2024

CI Failure Feedback

(Checks updated until commit 8eb2e4a)

Action: install-and-cache (ubuntu-latest, 3.11, 5)

Failed stage: Install django 5 [❌]

Failure summary:

The action failed due to the inability to activate the Python virtual environment. This was because
the script attempted to source .venv/bin/activate, but the .venv directory or the activate script
did not exist in the specified path.

Relevant error logs:
1:  ##[group]Operating System
2:  Ubuntu
...

625:  -rw-r--r--  1 runner docker   1709 Mar 31 19:34 .env.example
626:  -rw-r--r--  1 runner docker      0 Mar 31 19:34 __init__.py
627:  drwxr-xr-x  3 runner docker   4096 Mar 31 19:34 afb
628:  drwxr-xr-x 10 runner docker   4096 Mar 31 19:34 afbcore
629:  -rwxr-xr-x  1 runner docker    659 Mar 31 19:34 manage.py
630:  -rw-r--r--  1 runner docker 108574 Mar 31 19:34 poetry.lock
631:  drwxr-xr-x  3 runner docker   4096 Mar 31 19:34 static
632:  /home/runner/work/_temp/450b265d-92f2-44e0-9f35-316e8c103a55.sh: line 2: .venv/bin/activate: No such file or directory
633:  ##[error]Process completed with exit code 1.

✨ CI feedback usage guide:

The CI feedback tool (/checks) automatically triggers when a PR has a failed check.
The tool analyzes the failed checks and provides several feedbacks:

  • Failed stage
  • Failed test name
  • Failure summary
  • Relevant error logs

In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR:

/checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}"

where {repo_name} is the name of the repository, {run_number} is the run number of the failed check, and {job_number} is the job number of the failed check.

Configuration options

  • enable_auto_checks_feedback - if set to true, the tool will automatically provide feedback when a check is failed. Default is true.
  • excluded_checks_list - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list.
  • enable_help_text - if set to true, the tool will provide a help message with the feedback. Default is true.
  • persistent_comment - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true.
  • final_update_message - if persistent_comment is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true.

See more information about the checks tool in the docs.

"Deprecated. Please use the Project Settings to configure prebuilds.yaml-schema: Gitpod Configuration"
Copy link
Contributor Author

@delano delano left a comment

Choose a reason for hiding this comment

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

It might be beneficial to plan for future versions now by allowing more versions in ALLOWED_VERSIONS, such as ['v1', 'v2'], to make future transitions smoother. [medium]

We'll add a new version to list when it becomes available. Otherwise we'll need to handle requests to unsupported versions manually.

delano added 2 commits March 31, 2024 12:34
Signed-off-by: Delano <1206+delano@users.noreply.github.com>
Signed-off-by: Delano <1206+delano@users.noreply.github.com>
@delano delano merged commit 9e150c6 into dev Mar 31, 2024
0 of 2 checks passed
@delano delano deleted the delano/mar31-add-api-version branch March 31, 2024 19:35
@delano delano linked an issue Mar 31, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✔️ Done
Development

Successfully merging this pull request may close these issues.

Add version prefix to API path
1 participant