DEV Community

Pradeep Kalluri
Pradeep Kalluri

Posted on • Originally published at Medium

Rewriting My Apache Airflow PR: When Your First Solution Isn't the Right One

#python #dataengineering #opensource #programming

Lessons learned from contributing to Apache Airflow after getting my first PR merged - complete rewrite, 7 CI failures, and persistence

After getting my first Apache Airflow PR merged (#58587), I felt pretty confident about the contribution process. So when I found another bug, I jumped right in with what seemed like a perfect solution.

Two weeks and a complete rewrite later, my second PR (#59938) is now merged into Apache Airflow. Here's the real story—the good, the messy, and what changed.

🐛 Finding the Bug

It started with a production issue I encountered. Our Airflow scheduler kept crashing with a cryptic error:

InvalidStatsNameException: The stat name (pool.running_slots.data engineering pool 😎) 
has to be composed of ASCII alphabets, numbers, or the underscore, dot, or dash characters.
Enter fullscreen mode Exit fullscreen mode

Someone had created a pool with spaces and an emoji. Airflow accepted it, but when trying to report metrics, everything broke.

Having just gotten my first PR merged, I thought: "I know how this works now. I can fix this quickly."

Spoiler: It wasn't quick.

💻 My "Perfect" Solution: Validation

My approach seemed obvious:

  • Add validation when creating pools
  • Only allow ASCII letters, numbers, underscores, dots, and dashes
  • Reject invalid pool names with a clear error 
def validate_pool_name(name: str) -> None:
    if not re.match(r"^[a-zA-Z0-9_.-]+$", name):
        raise ValueError(
            f"Pool name '{name}' is invalid. Pool names must only contain "
            "ASCII alphabets, numbers, underscores, dots, and dashes."
        )
Enter fullscreen mode Exit fullscreen mode

I wrote tests, updated the news fragment, and submitted the PR with confidence.

Problem solved! Or so I thought.

💥 The Feedback That Humbled Me

@potiuk (Apache Airflow PMC member) reviewed my PR:

"I do not think it's a good idea to raise issue at pool creation time. This will mean that when you create an invalid pool, things will start crashing soon after. That's quite wrong behaviour."

He suggested a completely different approach: normalize the pool names for stats reporting instead of preventing them.

My heart sank. I'd spent hours on this validation approach, written tests, updated docs. But he was right:

The Problems With My Solution:

  • ❌ Users with existing "invalid" pools would be stuck
  • ❌ Migration would be complex and painful
  • ❌ It would break backward compatibility
  • ❌ Pools would be created but then crash the scheduler

The Better Approach:

  • ✅ Keep existing pools working
  • ✅ Normalize names only for stats reporting
  • ✅ Warn users, but don't break their systems
  • ✅ Graceful degradation instead of failures

Lesson 1: "Working" code isn't the same as "right" code.

My first PR was accepted with minor tweaks. This time, I needed to completely rethink the solution.

🔄 The Rewrite: Normalization

I threw away my validation code and started fresh:

def normalize_pool_name_for_stats(name: str) -> str:
    """
    Normalize pool name for stats reporting by replacing invalid characters.

    Stats names must only contain ASCII alphabets, numbers, underscores, 
    dots, and dashes. Invalid characters are replaced with underscores.
    """
    # Check if normalization is needed
    if re.match(r"^[a-zA-Z0-9_.-]+$", name):
        return name

    # Replace invalid characters with underscores
    normalized = re.sub(r"[^a-zA-Z0-9_.-]", "_", name)

    # Log warning
    logger.warning(
        "Pool name '%s' contains invalid characters for stats reporting. "
        "Reporting stats with normalized name '%s'. "
        "Consider renaming the pool to avoid this warning.",
        name,
        normalized,
    )

    return normalized
Enter fullscreen mode Exit fullscreen mode

Instead of preventing "bad" pool names, we:

  1. Accept any pool name (backward compatible)
  2. Normalize it when reporting metrics (fixes the crash)
  3. Log a warning (educates users)
  4. Suggest renaming (guides to best practices)

This was objectively better. And I would never have thought of it without the feedback.

Lesson 2: Maintainers see the bigger picture. Listen to them.

😤 The Static Check Marathon

I pushed my rewritten code. CI failed:

❌ Missing blank lines
❌ Import order wrong  
❌ LoggingMixin usage incorrect
❌ Missing 're' module import
Enter fullscreen mode Exit fullscreen mode

I fixed them. Pushed again. CI failed again with different formatting issues.

This happened SEVEN TIMES.

Each time:

  1. CI would auto-format and show what it wanted
  2. I'd try to apply the fixes manually (Windows, no local pre-commit)
  3. I'd push
  4. New formatting errors would appear

By attempt #5, I was frustrated. By attempt #7, I was questioning my career choices.

Lesson 3: Set up your local environment properly BEFORE you start coding.

✨ The Breakthrough

On attempt #8, I finally:

  1. Used PowerShell to apply exact formatting from CI diffs
  2. Added proper blank lines (2 after logger, 2 after functions)
  3. Fixed import order alphabetically
  4. Replaced LoggingMixin with module-level logger 
import logging

logger = logging.getLogger(__name__)


def normalize_pool_name_for_stats(name: str) -> str:
    # Function code...
    return normalized


class Pool(Base):
    # Class code...
Enter fullscreen mode Exit fullscreen mode

All checks passed! 🎉

@potiuk approved with "Two nits" (which I quickly fixed). Minutes later, the PR was merged into Apache Airflow's main branch.

Lesson 4: Persistence beats perfection. Keep going.

📊 First PR vs Second PR: The Differences

My First PR (#58587):

  • ⏱️ Time: 3 days
  • 🔄 Major rewrites: 0
  • ❌ CI failures: 2
  • 📝 Commits: 4
  • 🎓 Learned: The contribution process
  • ✅ Status: Merged

My Second PR (#59938):

  • ⏱️ Time: 2 weeks
  • 🔄 Major rewrites: 1 (complete approach change)
  • ❌ CI failures: 7
  • 📝 Commits: 16
  • 🎓 Learned: How to handle feedback, rewrites, and persistence
  • ✅ Status: Merged into Apache Airflow

The second one taught me WAY more.

📚 What I Learned (That My First PR Didn't Teach Me)

Technical Lessons:

1. Think About Backward Compatibility

My first solution would have broken existing users. Always ask: "What happens to people already using this?"

2. Graceful Degradation > Hard Failures

Warn users and normalize data instead of crashing. Systems should be resilient.

3. Pre-commit Hooks Are Non-Negotiable

Don't use CI as your formatter. Set up pre-commit locally FIRST.

4. Read Diffs Carefully

CI was telling me exactly what it wanted. I just needed to pay attention.

Soft Skills Lessons:

1. Be Ready to Throw Away Your Work

I spent hours on validation code. All of it went in the trash. That's okay. It's part of learning.

2. Feedback Isn't Personal

Potiuk wasn't criticizing me. He was helping me build something better. There's a huge difference.

3. Persistence Matters More Than Talent

7 CI failures felt embarrassing. But I kept going, and eventually it worked.

4. Document Your Thinking

In my PR description, I explained WHY I chose normalization after feedback. This helped reviewers understand my thought process.

🎯 Advice for Your Second (or Third, or Tenth) PR

When You Get Critical Feedback:

Don't:

  • ❌ Defend your solution immediately
  • ❌ Make minimal changes hoping they'll accept it
  • ❌ Take it personally

Do:

  • ✅ Read the feedback carefully (twice!)
  • ✅ Ask questions if you don't understand
  • ✅ Be willing to start over if needed
  • ✅ Thank reviewers for their time

When CI Keeps Failing:

Don't:

  • ❌ Push 10 commits trying to guess the fix
  • ❌ Ignore error messages
  • ❌ Give up after 3 failures

Do:

  • ✅ Set up local pre-commit hooks
  • ✅ Read the CI diff output carefully
  • ✅ Apply fixes locally and test before pushing
  • ✅ Ask for help if you're stuck after 3-4 attempts

When You Need to Rewrite:

Don't:

  • ❌ Try to salvage the old approach
  • ❌ Rush the rewrite
  • ❌ Skip tests because you're frustrated

Do:

  • ✅ Start with a clean slate
  • ✅ Think through the new approach carefully
  • ✅ Write better tests based on what you learned
  • ✅ Update documentation to match

🚀 Why You Should Keep Contributing

My first PR was smooth sailing. My second was rough waters.

That's exactly how learning works.

Each contribution teaches you something new:

  • First PR: The basics (fork, commit, PR, review)
  • Second PR: Handling feedback and major rewrites
  • Third PR: You'll discover this next!

For your career:

  • Real-world experience with design decisions
  • Proof you can handle feedback and pivot
  • Stories to tell in interviews ("I once had to completely rewrite my approach...")
  • Connections with senior engineers

For your skills:

  • Understanding trade-offs (validation vs normalization)
  • Production-quality code standards
  • Communication under pressure
  • Resilience and persistence

💡 Your Turn

If you've contributed once and it went well, do it again.

The second one will probably be harder. You might get asked to rewrite. CI might fail repeatedly. Reviewers might question your approach.

That's when the real learning happens.

My second PR took 4x longer than my first. It also taught me 10x more.

What will your next contribution teach you?

📎 Resources

My Merged PRs:

Helpful Links:

Originally published on Medium: https://medium.com/@kalluripradeep99/rewriting-my-apache-airflow-pr-when-your-first-solution-isnt-the-right-one-8c4243ca9daf

Top comments (0)