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

fix: don't expect /licenses to work for oss query service (#6763) #6826

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mgilham
Copy link

@mgilham mgilham commented Jan 16, 2025

Summary

fixes: #6763

The current code expects the /api/v2/licenses call to work, and will display an error message if it doesn't. But /api/v2/licenses is not a defined route in the OSS build, so the OSS build will always experience the error upon logging in.

This fix checks to see if we're running the OSS version, and if we are, ignores the error fetching licenses.

Related Issues / PR's

Screenshots

NA

Affected Areas and Manually Tested Areas


Important

Fixes error handling in OSS version by ignoring missing /licenses API error in AppRoutes/index.tsx.

  • Behavior:
    • In AppRoutes/index.tsx, added isOss check to determine if the application is running in OSS mode.
    • Modified error handling logic to ignore licensesFetchError if isOss is true, preventing unnecessary error display for missing /licenses API in OSS.

This description was created by Ellipsis for 87dc3e0. It will automatically update as commits are pushed.

@mgilham mgilham requested a review from YounixM as a code owner January 16, 2025 02:52
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 87dc3e0 in 1 minute and 2 seconds

More details
  • Looked at 24 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/AppRoutes/index.tsx:248
  • Draft comment:
    The comment about the OSS query service not having a licenses API is helpful but could be placed above the condition for better readability.
  • Reason this comment was not posted:
    Confidence changes required: 10%
    The logic for checking if the OSS version is running is correct, but the placement of the comment could be improved for clarity.
2. frontend/src/AppRoutes/index.tsx:227
  • Draft comment:
    Avoid using the component/index.tsx file structure approach, as it makes it difficult to debug and find components using global search tools like VS Code. This is applicable to other similar file structures in the project.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_JQb48I9YJbZ5LJKh


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v0.66.0-oss errors after login
1 participant