feat(api): retrieve app deployments based on last used data #7377
Conversation
Summary of ChangesHello @adambenhassen, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the API by providing a new GraphQL query, Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📚 Storybook DeploymentThe latest changes are available as preview in: https://pr-7377.hive-storybook.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new activeAppDeployments GraphQL query to find stale deployments, which is a valuable addition for maintenance and cleanup. The implementation is accompanied by thorough integration tests and clear documentation. My main feedback is a performance concern regarding how deployments are fetched and filtered, which could be problematic for targets with a large number of deployments. I've also noted an opportunity to reduce code duplication in the new tests.
packages/services/api/src/modules/app-deployments/providers/app-deployments.ts
Show resolved
Hide resolved
💻 Website PreviewThe latest changes are available as preview in: https://pr-7377.hive-landing-page.pages.dev |
|
🐋 This PR was built and pushed to the following Docker images: Targets: Platforms: Image Tag: |
adambenhassen
changed the title
adambenhassen
changed the title
adambenhassen
force-pushed
the
adam/console-1504
branch
from
7af3899 to
16aea1d
Compare
jdolle
left a comment
jdolle
left a comment
There was a problem hiding this comment.
Great job adding descriptions, comments, and docs. These are all very helpful. I'm excited for App Deployments to get more flushed out
| paginatedDeployments = filteredDeployments.filter(deployment => { | ||
| const deploymentCreatedAt = new Date(deployment.createdAt).getTime(); | ||
| return ( | ||
| deploymentCreatedAt < cursorCreatedAt || |
There was a problem hiding this comment.
I think it'd be simpler to build this into the sql query.
| AND "retired_at" IS NULL | ||
| ${args.filter.name ? sql`AND "name" ILIKE ${'%' + args.filter.name + '%'}` : sql``} | ||
| ORDER BY "created_at" DESC, "id" | ||
| LIMIT ${maxDeployments} |
There was a problem hiding this comment.
To avoid setting this limit and to simplify this logic, I wonder if we should be flip the query order. So that you first collect, from Clickhouse, a set of the deployments that match the lastUsed filter, and then use those IDs to fetch the rest of the details from postgres. This should eliminate the overfetching and avoid any limit issues for incredibly large deployment sets
2 participants
Description
Adds
activeAppDeploymentsGraphQL query to find app deployments based on usage criteria, enabling teams to identify and clean up stale deploymentsFilter options:
lastUsedBefore- find deployments used but not recentlyneverUsedAndCreatedBefore- find old unused deploymentsname- filter by app name (partial match)Closes CONSOLE-1504