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

Update Event Docs #6334

Merged
merged 1 commit into from
Mar 12, 2025
Merged

Conversation

CtfChan
Copy link
Contributor

@CtfChan CtfChan commented Mar 12, 2025

Tracking issue

Closes #3663

Why are the changes needed?

Documentation was incorrect. For GCP you need projectId instead of region.

What changes were proposed in this pull request?

Setup correct Helm values for GCP.

How was this patch tested?

Tested in CI. Updated a few lines in docs.

Labels

changed

Setup process

N/A

Screenshots

N/A

Check all the applicable boxes

  • I updated the documentation accordingly.
  • All new and existing tests passed.
  • All commits are signed-off.

Related PRs

N/A

Docs link

N/A

Summary by Bito

This PR updates event documentation by fixing configuration parameters and enhancing clarity. It removes outdated AWS examples, introduces a tabbed layout for AWS SNS and GCP Pub/Sub, and correctly uses projectId for GCP instead of region. These changes improve documentation accuracy and usability.

Unit tests added: False

Estimated effort to review (1-5, lower is better): 1

Copy link

welcome bot commented Mar 12, 2025

Thank you for opening this pull request! 🙌

These tips will help get your PR across the finish line:

  • Most of the repos have a PR template; if not, fill it out to the best of your knowledge.
  • Sign off your commits (Reference: DCO Guide).

@flyte-bot
Copy link
Collaborator

flyte-bot commented Mar 12, 2025

Code Review Agent Run #768a6d

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: d40402e..d40402e
    • docs/deployment/configuration/cloud_event.rst
    • docs/deployment/configuration/eventing.rst
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

@flyte-bot
Copy link
Collaborator

flyte-bot commented Mar 12, 2025

Changelist by Bito

This pull request implements the following key changes.

Key Change Files Impacted
Documentation - Documentation Updates

cloud_event.rst - Replaced AWS region with GCP projectId in cloud event configuration.

eventing.rst - Refactored eventing documentation by removing outdated AWS examples and adding a new tabbed layout with updated AWS SNS and GCP Pub/Sub configurations.

Signed-off-by: Chris <[email protected]>
@CtfChan CtfChan force-pushed the ctfchan/fix-event-docs branch from d40402e to 4de2108 Compare March 12, 2025 05:15
@flyte-bot
Copy link
Collaborator

flyte-bot commented Mar 12, 2025

Code Review Agent Run #2fd5de

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 4de2108..4de2108
    • docs/deployment/configuration/cloud_event.rst
    • docs/deployment/configuration/eventing.rst
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by Bito Logo

@eapolinario eapolinario enabled auto-merge (squash) March 12, 2025 20:44
Copy link
Contributor

@eapolinario eapolinario left a comment

Choose a reason for hiding this comment

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

Thank you.

Copy link

codecov bot commented Mar 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 58.49%. Comparing base (1dca058) to head (4de2108).
Report is 6 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6334   +/-   ##
=======================================
  Coverage   58.48%   58.49%           
=======================================
  Files         937      937           
  Lines       71091    71091           
=======================================
+ Hits        41580    41586    +6     
+ Misses      26359    26353    -6     
  Partials     3152     3152           
Flag Coverage Δ
unittests-datacatalog 59.06% <ø> (ø)
unittests-flyteadmin 56.30% <ø> (+0.02%) ⬆️
unittests-flytecopilot 30.99% <ø> (ø)
unittests-flytectl 64.70% <ø> (ø)
unittests-flyteidl 76.12% <ø> (ø)
unittests-flyteplugins 61.00% <ø> (ø)
unittests-flytepropeller 54.80% <ø> (ø)
unittests-flytestdlib 64.04% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@eapolinario eapolinario merged commit eeac101 into flyteorg:master Mar 12, 2025
51 checks passed
Copy link

welcome bot commented Mar 12, 2025

Congrats on merging your first pull request! 🎉

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.

[BUG] Sending cloud events from flyteadmin not documented properly for GCP and results in errors
3 participants