-
Notifications
You must be signed in to change notification settings - Fork 7
Migration PR: Staging to main #86
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
Conversation
rename project and stack --------- Co-authored-by: sourabhlodha <sourabhlodha@Administrators-MacBook-Pro.local>
* use latest docker image * update envsample
* token expiry time can be customize * default to one day
* trial * pushing all * models file * renaming * Rename Project.py to project.py * Rename oganization.py to organization.py * Update README.md (#44) * changes (#45) Co-authored-by: sourabhlodha <sourabhlodha@Administrators-MacBook-Pro.local> * Readme update (#47) rename project and stack --------- Co-authored-by: sourabhlodha <sourabhlodha@Administrators-MacBook-Pro.local> * fix create_user endpoint (#62) * standard api response and http exception handling (#67) * standardization and edits * small edits * small edits * small edits * fixed project post * trial * pushing all * models file * renaming * Rename Project.py to project.py * Rename oganization.py to organization.py * standardization and edits * small edits * small edits * small edits * fixed project post * remove these files since they were somehow pushed into this branch * re-push the docker file * re-push utils file * re-push the file * fixing test cases --------- Co-authored-by: Sourabh Lodha <sourabh_lodha@ymail.com> Co-authored-by: sourabhlodha <sourabhlodha@Administrators-MacBook-Pro.local> Co-authored-by: Aviraj Gour <100823015+avirajsingh7@users.noreply.github.com> Co-authored-by: Ishankoradia <ikoradia@umich.edu>
* intial commit user project mapping and authorization * fix alembic migration * Use standard API response * add pagination * add index and use base model
* fixing testcases and migrations * changes migration file name * remove old migration --------- Co-authored-by: Akhilesh Negi <akhileshnegi.an3@gmail.com>
* Intial setup api key * added Api key auth flow * support both api key and oauth --------- Co-authored-by: Sourabh Lodha <sourabh_lodha@ymail.com>
Back merge Production to staging code
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
|
||
|
|
||
| # Add a user to a project | ||
| @router.post("/{user_id}", response_model=APIResponse[ProjectUserPublic]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from fastapi import RateLimiter
@app.state.limiter.limit('5/minute')
@router.post('/{user_id}', response_model=APIResponse[ProjectUserPublic])Add rate limiting to protect endpoints from potential abuse
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
|
|
||
| # Shared properties for an Organization | ||
| class OrganizationBase(SQLModel): | ||
| name: str = Field(unique=True, index=True, max_length=255) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name: str = Field(unique=True, index=True, max_length=255, min_length=1, regex=r'^[\w\s\-\.\&\,\']+$')Add input validation for organization name.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
Code Review Completed! 🔥The code review was successfully completed based on your current configurations. Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
| env: | ||
| REGISTRY: ${{ steps.login-ecr.outputs.registry }} | ||
| REPOSITORY: ${{ github.event.repository.name }}-repo | ||
| TAG: ${{ github.ref_name }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
env:
REGISTRY: ${{ steps.login-ecr.outputs.registry }}
REPOSITORY: ${{ github.event.repository.name }}-repo
TAG: ${{ github.ref_name }}
run: |
if [ -z "$REGISTRY" ] || [ -z "$REPOSITORY" ] || [ -z "$TAG" ]; then
echo "Required environment variables are not set"
exit 1
fiAdd environment variable validation to ensure required variables are set before running critical operations.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| aws ecs update-service \ | ||
| --cluster ${{ github.event.repository.name }}-cluster \ | ||
| --service ${{ github.event.repository.name }}-service \ | ||
| --force-new-deployment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aws ecs update-service \n --cluster ${{ github.event.repository.name }}-cluster \n --service ${{ github.event.repository.name }}-service \n --force-new-deployment
aws ecs wait services-stable \n --cluster ${{ github.event.repository.name }}-cluster \n --services ${{ github.event.repository.name }}-service \n --timeout-seconds 300Add timeout configuration for ECS deployment to prevent indefinite hanging of the workflow in case of deployment issues.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| - name: Configure AWS credentials | ||
| uses: aws-actions/configure-aws-credentials@v4 # More information on this action can be found below in the 'AWS Credentials' section | ||
| with: | ||
| role-to-assume: arn:aws:iam::024209611402:role/github-action-role |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
role-to-assume: arn:aws:iam::${{ secrets.AWS_ACCOUNT_ID }}:role/github-action-roleHard-coded AWS account ID should be stored as a GitHub secret for better security
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
| ENV PYTHONUNBUFFERED=1 | ||
|
|
||
| # Set working directory | ||
| WORKDIR /app/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WORKDIR /app/
RUN adduser --system --no-create-home appuser
RUN chown -R appuser:appuser /app
USER appuserAdd a non-root user and switch to it for better security. Running containers as root is a security risk that should be avoided in production environments.
Talk to Kody by mentioning @kody
Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.
* fix authentication part * Modify test cases to compatible with new auth
* issue CI * first stab at continuous integration * fixing testcases and migrations * syncing with master * moving to python version 3.11.7 * making copy of env * updating env * added migrations * added uv sync * updating working directory * added step to activate env * updating working directory * updating working directory for codecov upload * updating script to upload to codecov * remove working directory * added working directory for % check * clenaup * cleanup * activating env * update the issue template * update readme and env file
jerome-white
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are far too many files, and far too many purposes of those files, to give an in-depth review. Also, my understanding is that this is effectively a branch move, implying that most of these files have been reviewed already.
There are several comments from the review bot. They mostly seem like defensive programming suggestions. Whether to implement them should be the decision of those that have more context. I'm comfortable making them into low priority issues and fixing them one-by-one at a later date.
Summary
Target issue is #87
This pull request merges changes from the
staging-to-mainbranch into themainbranch of the ProjectTech4DevAI/ai-platform repository. The updates include significant enhancements and new features across various components of the FastAPI application:API Enhancements:
deps.py.main.py.organization.pyandproject.py.project_user.py.login.py.Core and Configuration Updates:
config.pyfor enhanced security.main.py.Model and CRUD Operations:
organization.pyandproject_user.py.Utility and Documentation Improvements:
utils.py.Miscellaneous:
users.pyto avoid naming conflicts.envSamplefor project configuration.These changes aim to enhance the functionality, security, and maintainability of the AI Platform project.
This pull request, titled "Migration PR: Staging to main," merges changes from the
staging-to-mainbranch into themainbranch of the ProjectTech4DevAI/ai-platform repository. The updates include:GitHub Actions Workflows:
Backend Updates:
Model Enhancements:
Documentation:
Configuration:
These changes aim to enhance the deployment process, improve code organization, and strengthen security and authentication mechanisms.