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

Win 465 index observability 1 #4538

Merged
merged 4 commits into from
Oct 16, 2024
Merged

Conversation

wendrul
Copy link
Contributor

@wendrul wendrul commented Oct 16, 2024

https://github.com/windmill-labs/windmill-ee-private/pull/75
Screenshot from 2024-10-16 18-58-29


Important

Centralized isJobCancelable function in utils.ts and enhanced GlobalSearchModal.svelte to display index metadata and lock ownership status.

  • Refactoring:
    • Centralized isJobCancelable function in utils.ts and removed duplicate definitions from RunRow.svelte, RunsTable.svelte, and +page.svelte.
  • Enhancements:
    • GlobalSearchModal.svelte now displays indexMetadata and lost_lock_ownership status in the job preview section.
  • Misc:
    • Updated imports in RunRow.svelte, RunsTable.svelte, and +page.svelte to use centralized isJobCancelable function.

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

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 6ab3c1b in 22 seconds

More details
  • Looked at 151 lines of code in 5 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. frontend/src/lib/utils.ts:20
  • Draft comment:
    Ensure that the additional condition !j.canceled in isJobCancelable is intended and does not cause any unintended behavior. This condition was not present in the original function definitions in the other files.
  • Reason this comment was not posted:
    Comment did not seem useful.

Workflow ID: wflow_G5kh0a09uVKOdkV5


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

Copy link

cloudflare-workers-and-pages bot commented Oct 16, 2024

Deploying windmill with  Cloudflare Pages  Cloudflare Pages

Latest commit: d609140
Status: ✅  Deploy successful!
Preview URL: https://e43e8f52.windmill.pages.dev
Branch Preview URL: https://win-465-index-observability.windmill.pages.dev

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! Incremental review on d609140 in 22 seconds

More details
  • Looked at 7 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. backend/ee-repo-ref.txt:1
  • Draft comment:
    The change here is a simple update of a reference hash, which is typical for updating a dependency or a submodule reference. No issues found.
  • Reason this comment was not posted:
    Confidence changes required: 0%
    The change in the file is a simple update of a reference hash, which is typical for updating a dependency or a submodule reference. This doesn't require any changes or comments as it is a standard practice.

Workflow ID: wflow_GcCyFcIlP6z7yXTG


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

@rubenfiszel rubenfiszel merged commit c06c42a into main Oct 16, 2024
4 checks passed
@rubenfiszel rubenfiszel deleted the win-465-index-observability-1 branch October 16, 2024 17:25
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 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.

2 participants