Skip to content

[Testing]Temp - #3175

Closed
agsaru wants to merge 4 commits into
Netflix:masterfrom
agsaru:temp
Closed

[Testing]Temp#3175
agsaru wants to merge 4 commits into
Netflix:masterfrom
agsaru:temp

Conversation

@agsaru

@agsaru agsaru commented May 5, 2026

Copy link
Copy Markdown
Contributor

PR Type

  • Bug fix
  • New feature
  • Core Runtime change (higher bar -- see CONTRIBUTING.md)
  • Docs / tooling
  • Refactoring

Summary

Issue

Fixes #

Reproduction

Runtime:

Commands to run:

# paste exact commands

Where evidence shows up:

Before (error / log snippet)
paste here
After (evidence that fix works)
paste here

Root Cause

Why This Fix Is Correct

Failure Modes Considered

Tests

  • Unit tests added/updated
  • Reproduction script provided (required for Core Runtime)
  • CI passes
  • If tests are impractical: explain why below and provide manual evidence above

Non-Goals

AI Tool Usage

  • No AI tools were used in this contribution
  • AI tools were used (describe below)

Copilot AI review requested due to automatic review settings May 5, 2026 14:39
@greptile-apps

greptile-apps Bot commented May 5, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This is a temporary testing PR with two minor devtools changes. The tiltfile refactor correctly inlines a constant (_sfn_host_network was hardcoded to False in every branch) with no behavioral change, and the eventbridge stub receives a placeholder comment that should be cleaned up.

  • sfn_local.tiltfile: Removes the _sfn_host_network variable (always False) and inlines hostNetwork: False and dnsPolicy: 'ClusterFirst' directly — the dnsPolicy ternary previously always resolved to 'ClusterFirst', so the logic is unchanged.
  • eventbridge_stub.py: Appends a # Added Hello comment at the end of the file with no informational value.

Confidence Score: 5/5

Safe to merge as a temporary devtools-only change; neither file touches runtime logic.

Both changes are confined to the local development tooling (devtools/). The tiltfile simplification is a correct constant-folding with no observable effect, and the only other change is a meaningless comment. There is no impact on Metaflow runtime, production code, or tests.

No files require special attention, though the placeholder # Added Hello comment in eventbridge_stub.py should be removed before promoting this branch.

Important Files Changed

Filename Overview
devtools/eventbridge_stub.py Only change is appending a # Added Hello comment with no purpose — likely temporary testing noise.
devtools/tilt/sfn_local.tiltfile Removes the always-False _sfn_host_network variable and inlines the constant values — a correct simplification with no behavioral change.

Reviews (2): Last reviewed commit: "added hello" | Re-trigger Greptile

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR cleans up the Tilt configuration for running Step Functions Local by removing an unused hostNetwork toggle and hard-coding the Kubernetes pod networking settings to the existing behavior (no host networking).

Changes:

  • Removes the unused _sfn_host_network variable/assignments.
  • Sets hostNetwork: False and dnsPolicy: ClusterFirst directly in the Deployment spec.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +36 to +37
'hostNetwork': False,
'dnsPolicy': 'ClusterFirst',
@agsaru agsaru closed this May 5, 2026
@agsaru
agsaru deleted the temp branch May 5, 2026 14:46
@codecov

codecov Bot commented May 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (master@4ab2fc0). Learn more about missing BASE report.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #3175   +/-   ##
=========================================
  Coverage          ?   27.48%           
=========================================
  Files             ?      376           
  Lines             ?    51827           
  Branches          ?     9166           
=========================================
  Hits              ?    14246           
  Misses            ?    36676           
  Partials          ?      905           

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants