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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix list jobs and intervals mismatch #3767

Merged
merged 7 commits into from
May 21, 2024

Conversation

wendrul
Copy link
Contributor

@wendrul wendrul commented May 17, 2024

  • Refactor frontend to only use the new endpoint when filtering on concurrency keys.
  • Change backend endpoint to return the same as the list_jobs endpoint (+ filtering and obscured jobs)
  • Small fixes
馃殌 This description was created by Ellipsis for commit 7ca08be

Summary:

Refactored job listing and concurrency handling in the backend and updated frontend components to use the new API structure.

Key points:

  • Renamed /intervals endpoint to /list_jobs in concurrency_groups.rs.
  • Updated schemas in openapi.yaml to reflect new endpoint and data structures.
  • Refactored backend logic in concurrency_groups.rs for new endpoint and data handling.
  • Adjusted frontend components in ConcurrentJobsChart.svelte, JobLoader.svelte, and RunsTable.svelte to use updated API.
  • Improved error handling and data fetching in frontend components.

Generated with 鉂わ笍 by ellipsis.dev


馃殌 This description was created by Ellipsis for commit 6b08034

Summary:

Refactored job listing and concurrency handling in the backend and updated frontend components to use the new API structure, with enhanced data privacy features and improved efficiency.

Key points:

  • Refactor frontend to only use the new endpoint when filtering on concurrency keys.
  • Change backend endpoint to return the same as the list_jobs endpoint (+ filtering and obscured jobs)
  • Small fixes
  • Refactor get_concurrent_intervals in concurrency_groups.rs to handle job listings with concurrency keys and user permissions.
  • Implement obscured job handling in the backend to ensure data privacy based on user permissions.
  • Refactored backend handling of job listings and concurrency in /concurrency_groups.rs and /jobs.rs.
  • Updated frontend components to use new API endpoints for job listings.
  • Improved handling of obscured jobs to enhance data privacy.
  • Streamlined backend queries for efficiency.

Generated with 鉂わ笍 by ellipsis.dev

Fix a mismatch between to endpoints when filtering by concurrency key
@wendrul wendrul marked this pull request as draft May 17, 2024 18:23
Copy link

cloudflare-pages bot commented May 17, 2024

Deploying windmill with 聽Cloudflare Pages 聽Cloudflare Pages

Latest commit: 4dd50a0
Status:鈿★笍聽 Build in progress...

View logs

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 fb0a6c7 in 4 minutes and 18 seconds

More details
  • Looked at 1107 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_dOyg2PeMQseZ8TnX


You can customize Ellipsis with 馃憤 / 馃憥 feedback, review rules, user-specific overrides, quiet mode, and more.

@wendrul wendrul marked this pull request as ready for review May 21, 2024 08:48
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 7ca08be in 3 minutes and 45 seconds

More details
  • Looked at 1171 lines of code in 7 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_MRLO2sOnUkRvgq8s


You can customize Ellipsis with 馃憤 / 馃憥 feedback, review rules, user-specific overrides, quiet mode, and more.

));
}

const QJ_FIELDS: &[&str] = &[
Copy link
Contributor

Choose a reason for hiding this comment

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

is that different from the ones of list_jobs ? Can we make it a common const if yes

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! Incremental review on c11e1a2 in 4 minutes and 41 seconds

More details
  • Looked at 45 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. backend/windmill-api/src/concurrency_groups.rs:394
  • Draft comment:
    Consider renaming get_concurrent_intervals to get_concurrent_jobs to align with the new endpoint naming convention (/list_jobs). This will improve consistency and readability.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_O0Srw3q5Ci5LBgE7


You can customize Ellipsis with 馃憤 / 馃憥 feedback, review rules, user-specific overrides, quiet mode, and more.

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! Incremental review on 6b08034 in 2 minutes and 38 seconds

More details
  • Looked at 193 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. backend/windmill-api/src/jobs.rs:936
  • Draft comment:
    The function filter_list_queue_query is quite complex and handles many different conditions for filtering SQL queries. Consider refactoring this function to improve readability and maintainability. Breaking it down into smaller, more focused functions could help manage complexity, especially as new filters might be added in the future.
  • Reason this comment was not posted:
    Confidence of 0% on close inspection, compared to threshold of 50%.

Workflow ID: wflow_Rp1ZBtfbtMttM7b6


You can customize Ellipsis with 馃憤 / 馃憥 feedback, review rules, user-specific overrides, quiet mode, and more.

@rubenfiszel rubenfiszel merged commit 1001b1e into main May 21, 2024
3 of 4 checks passed
@rubenfiszel rubenfiszel deleted the win-136-fix-list-jobs-and-intervals-mismatch branch May 21, 2024 10:21
@github-actions github-actions bot locked and limited conversation to collaborators May 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants