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.

Sign up for GitHub

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

Jump to bottom

feat(platform/featureflags): Setting up feature flagging #8718

Merged
merged 22 commits into from
Nov 27, 2024
Merged

feat(platform/featureflags): Setting up feature flagging #8718

merged 22 commits into from
Nov 27, 2024

Conversation

aarushik93
Copy link
Contributor

@aarushik93 aarushik93 commented Nov 19, 2024 •
edited
Loading

So we can hide and show features at will

Changes 🏗️

Created a launch darkly decorator that can be used to hide or show functionality in the backend.

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • ...
Example test plan
  • Create from scratch and execute an agent with at least 3 blocks
  • Import an agent from file upload, and confirm it executes correctly
  • Upload agent to marketplace
  • Import an agent from marketplace and confirm it executes correctly
  • Edit an agent from monitor, and confirm it executes correctly

For configuration changes:

  • .env.example is updated or already compatible with my changes
  • docker-compose.yml is updated or already compatible with my changes
  • I have included a list of my configuration changes in the PR description (under Changes)
Examples of configuration changes
  • Changing ports
  • Adding new services that need to communicate with each other
  • Secrets or environment variable changes
  • New or infrastructure changes such as databases

@github-actions github-actions bot added platform/backend AutoGPT Platform - Back end conflicts Automatically applied to PRs with merge conflicts labels Nov 19, 2024
@github-actions GitHub Actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@netlify Netlify
Copy link

netlify bot commented Nov 19, 2024 •
edited
Loading

✅ Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 106b077
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67477210965100000811ff80

@aarushik93 aarushik93 force-pushed the aarushikansal/feature-flagging-backend branch from 141bc46 to 4e09cef Compare November 20, 2024 17:42
@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Nov 20, 2024
@github-actions GitHub Actions
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions bot added size/xl and removed size/l labels Nov 20, 2024
@aarushik93 aarushik93 marked this pull request as ready for review November 20, 2024 18:12
@aarushik93 aarushik93 requested a review from a team as a code owner November 20, 2024 18:12
@aarushik93 aarushik93 requested review from Swiftyos, Bentlybro, ntindle and kcze and removed request for a team November 20, 2024 18:12
@qodo-merge-pro Qodo Merge Pro
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 Security concerns

Sensitive information exposure:
The code logs SDK key presence at debug level (line 28 in client.py), which could potentially expose sensitive information in logs if the actual key is accidentally logged in other debug statements. Consider removing SDK key related debug logging.

⚡ Recommended focus areas for review

Error Handling
The error handling in the feature flag decorator returns None by default when an exception occurs, which could lead to unexpected behavior. Consider raising the exception or providing a more specific error response.

Code Duplication
The sync_wrapper and async_wrapper functions contain duplicated logic for feature flag checking. Consider extracting common logic to reduce duplication.

Logging Level
Setting logging level to DEBUG by default may be too verbose for production environments and could impact performance.

@aarushik93 aarushik93 enabled auto-merge (squash) November 20, 2024 18:24
@aarushik93 aarushik93 disabled auto-merge November 20, 2024 19:43
@aarushik93 aarushik93 requested review from ntindle and Pwuts November 26, 2024 20:28
@aarushik93
Copy link
Contributor Author

Ran the formatter against autogpt_libs - hence why the extra changes in this PR now!

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Nov 26, 2024
@github-actions GitHub Actions
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

ntindle
ntindle previously approved these changes Nov 26, 2024
View reviewed changes
@aarushik93 aarushik93 enabled auto-merge (squash) November 26, 2024 22:42
@netlify Netlify
Copy link

netlify bot commented Nov 26, 2024 •
edited
Loading

✅ Deploy Preview for auto-gpt-docs-dev canceled.

Name Link
🔨 Latest commit 106b077
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs-dev/deploys/67477210cad1b60008e1e831

@github-actions GitHub Actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Nov 26, 2024
@aarushik93 aarushik93 disabled auto-merge November 27, 2024 12:59
@github-actions GitHub Actions
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions bot added conflicts Automatically applied to PRs with merge conflicts and removed conflicts Automatically applied to PRs with merge conflicts labels Nov 27, 2024
@github-actions GitHub Actions
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions GitHub Actions
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Nov 27, 2024
@aarushik93 aarushik93 merged commit 4b8087c into dev Nov 27, 2024
19 checks passed
@aarushik93 aarushik93 deleted the aarushikansal/feature-flagging-backend branch November 27, 2024 19:29
aarushik93 added a commit that referenced this pull request Dec 1, 2024
@aarushik93
feat(platform/featureflags): Setting up feature flagging (#8718)
07bd933 
* feature flag formatting and linting

* add tests

* update poetry lock

* remove unneeded changes

* fix pyproject

* fix formatting and linting

* pydantic settings

* address comments and format

* alphabetize

* fix lockfile

* fix conflicts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ntindle ntindle ntindle left review comments

@Swiftyos Swiftyos Awaiting requested review from Swiftyos Swiftyos is a code owner automatically assigned from Significant-Gravitas/maintainers

@Bentlybro Bentlybro Awaiting requested review from Bentlybro Bentlybro is a code owner automatically assigned from Significant-Gravitas/maintainers

@kcze kcze Awaiting requested review from kcze

@Pwuts Pwuts Awaiting requested review from Pwuts

Assignees
No one assigned
Labels
platform/backend AutoGPT Platform - Back end Review effort [1-5]: 3 size/xl
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aarushik93 @ntindle @Pwuts