Oleksandr Kuzminskyi
@akuzminsky · Infrastructure & Database Engineer | AWS + Terraform + Python | I build ISO 27001-ready clouds for startups | Tinker with home automation for fun.
direct and technical with helpful specificity
Highly technical and thorough with a focus on production safety and infrastructure best practices. Provides detailed, structured feedback with specific code examples and clear reasoning. Known for comprehensive reviews that can be quite lengthy but are extremely valuable for preventing production issues.
3519
Comments
2857
PRs
66
Repos
135
Avg Chars
7
Harshness
Personality
Infrastructure security conscious
Production-safety focused
Detail-oriented and thorough
Direct communicator
Best practices advocate
Performance optimization minded
Risk mitigation focused
Helpful with specific examples
Greatest Hits
"checks are failing"
"We need to move the job configuration out of the workflow file. Otherwise, you will need an approval from the security-team."
"btw, to avoid the clean-up step, you can run `docker run --rm`. This will delete the container after a run."
"We should not pass a password in an environment variable. Instead, the code should fetch it from secrets manager."
"What are we changing here"
"Please make it reviewable. The PR looks raw."
"can I terminate it then? why would we pay $$ for something we don't use?"
Focus Areas
- Infrastructure security
- Production safety
- Docker optimization
- CI/CD workflows
- AWS configurations
- Performance optimization
- Error handling
- Configuration management
- Best practices enforcement
Common Phrases
"We need to"
"We should"
"This will"
"Before we"
"I suggest"
"checks are failing"
"What is the"
"btw,"
"can you"
"Do you want to"
"How did"
"any insights"
"What happens when"
"There is a"
"I found that"
Sentiment Breakdown
Review Outcomes
Most Reviewed Authors
Spiciest Comments
I know, the matrix gets popular recently around.
However, it doesn't work well in the CD scenario.
What we need.
We need a subsequent execution of the CD job in sandbox and only after then, if it was successful in sandbox, run it in production.
How can you implement it?
> @akuzminsky is there a way to force MODIFY the column instead of dropping it and adding a new one?
@colriot skeema doesn't support it natively.
> **Renaming columns or tables**
> Skeema cannot currently be used to rename columns within a table, or to rename entire tables. This is a shortcoming of Skeema’s declarative approach: by expressing everything as a CREATE TABLE, there is no way for Skeema to know (with absolute certainty) the difference between a column rename vs dropping an exist
# PR #155 Review: Add ECR Image Tag Validation to Terraform CI Pipeline
**Last Updated: 2026-03-04**
**Reviewer: terraform-module-reviewer**
**Branch: eva-stability/ci-ecr-validation**
**Closes: INF-1091**
---
## Executive Summary
This PR adds a pre-plan shell script (`validate-ecr-tag.sh`) invoked from both CI jobs that checks whether the `eva_docker_image_label` tag value in `environments/<env>/main.tf` actually exists in ECR before Terraform is allowed to plan. The motivation i
It can be done and I just did without a third-party provider.
I don't really understand the "needed downstream" part, it doesn't add up with what I know about the public key authentication, but ok.
And finally, deploy key based deployments is an anti-pattern.
Anyway, I implemented what you originally requested. And if it doesn't work as needed (see my concern above), then let's revisit to find a better deployment process.
Why would `terraform.tf` change?
@zifanwTF the pr is GTG
oh, the code formatting check has failed. @zifanwTF , please run `make format`.
> > Comment was too big. It's published as a gist at https://gist.github.com/shuhaodo/6f1837d75bf6455c67878ed46e752bc1.
>
> What do you suggest me to do?
@zifanwTF the plan looks good. feel free to merge the PR
plan review from CC:
● I've analyzed the plan carefully. Here are the dangerous and concerning changes:
🔴 CRITICAL ISSUES
1. RDS Identifier Rename - BRIEF DOWNTIME EXPECTED
```
~ identifier = "wiki-encrypted20250205232235963600000001" -> "wiki-encrypted"
```
Impact: This will cause a 1-5 minute outage of your BookStack wiki while AWS renames the RDS instance.
Why: The module README explicitly warns about this:
"DOWNTIME AVOIDANCE: When upgrading from v2.x, RDS will r
> @akuzminsky um cron policy will be removed in near future, but policy statement should be same
why would you remove it? There is a policy.
It's used by um_cron instance. Now, that you migrate the job to action runners, it will be used by the action runner job.
AI Persona Prompt
You are @akuzminsky, a senior infrastructure engineer with deep expertise in AWS, Docker, CI/CD, and production systems. Your reviews are comprehensive, technical, and focused on preventing production issues. You have a direct communication style and aren't afraid to point out problems, but you always provide specific, actionable solutions with code examples.
Key characteristics of your review style:
- Start simple comments with lowercase and keep them concise when possible
- Use 'btw,' frequently for helpful asides
- Ask direct questions like 'What is the...?' or 'How did...?' when something is unclear
- Frequently say 'We need to' or 'We should' when suggesting changes
- Call out 'checks are failing' immediately when you see CI issues
- Focus heavily on security, especially around secrets management and IAM permissions
- Optimize Docker images obsessively - suggest specific base images, layer consolidation, and cleanup commands
- Catch configuration drift and hardcoded values that should be parameterized
- Write detailed technical explanations when you find complex issues, sometimes with severity classifications
- Question unnecessary complexity or costs with phrases like 'why would we pay $$ for something we don't use?'
- Provide specific code snippets and examples in your suggestions
- Be particularly strict about production safety and infrastructure best practices
You review infrastructure code, workflows, and configurations with the mindset of preventing outages and security issues. You're helpful but demanding, and you expect PRs to be production-ready before approval.
Recent Comments (867 total)
Is there a PR in torchx for urllib3 update?
for the record https://github.com/pytorch/torchx/issues/1040
Add on pull request to test the workflow while it's not merged yet
can you limit the run if files in "apps/google_hotel_sqs" change?
Before we put the producer in the cron, we should test the consumer first. I suggest we remove the workflow files from this PR and add them later.
Nonetheless, I'll leave my comments to address them later in the subsequent PR.
We need to move the job configuration out of the workflow file. Otherwise, you will need an approval from the security-team. We can come back to this issue until after the migration though.
This job should not build the image. We will build in the CD step, when we build the consumer image. This job will just pull the image and run a container.
btw, to avoid the clean-up step, you can run `docker run --rm`. This will delete the container after a run.
I found that official Python "slim" images yield smaller image. The difference is substantial. Which is important for autoscaling consumers.
```
FROM python:3.13-slim
```
I would build locally from different bases and compare the resulting image sizes.
Run apt stuff in one layer. It will producer a smaller image. And a clean-up helps, too.
e.g.
```
RUN apt-get update && \
apt-get install -y --no-install-recommends \
easy-rsa \
procps \
&& \
apt-get clean && \
rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*
```
I suspect, a clean-up is needed here, too. `playwright install` runs apt-get behind the scene.
```
RUN poetry run playwright install --with-deps && \
apt-get clean && \
rm -rf /var/lib/apt/lists/* /tmp/* /var/tmp/*
```
I experimented with different bases:
debian:12
```
google-hotels latest d14b52c8773f 15 seconds ago 3.52GB
```
python:3.11-slim-bookworm
```
google-hotels latest 66f88da40265 7 seconds ago 3.02GB
```
python:3.11-slim-bookworm with cleanup
```
google-hotels
The best result was with this Dockerfile
```
$ git diff
diff --git a/apps/google_hotel_sqs/Dockerfile b/apps/google_hotel_sqs/Dockerfile
index bd11e1c..1a54250 100644
--- a/apps/google_hotel_sqs/Dockerfile
+++ b/apps/google_hotel_sqs/Dockerfile
@@ -1,13 +1,13 @@
-FROM debian:12
+FROM python:3.11-slim-bookworm
-RUN apt-get update && apt-get upgrade -y && apt-get install -y curl openssl
The biggest layer is playwright . It's ~2G. Maybe if we don't need all browsers that it installs by default, we can install only ones we need.
```
requests==2.32.3
```
looks like it's not happy about requests. Any insights?