fix(env): export dest 末尾 / のファイル名自動補完 (#24)#25
Open
takemi-ohama wants to merge 2 commits into
Open
Conversation
`devbase env export s3://bucket/prefix/` のように末尾 `/` の dest を指定したとき、 ファイル名部分が空のオブジェクトが作られていた問題を修正する。 `aws s3 cp` 互換でディレクトリ的 dest に既定ファイル名 (`devbase-env-<TS>.dbenv`) を自動補完する。 - S3 URI 末尾 `/`: prefix にファイル名を append - ローカル既存ディレクトリ / 末尾 `/`: ディレクトリ配下にファイルを生成 - それ以外 (フルキー / 通常ファイルパス / stdio `-`) は従来通り Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
takemi-ohama
commented
May 24, 2026
Contributor
Author
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 1 | codex | APPROVE
修正必須の指摘事項はありません。
takemi-ohama
commented
May 24, 2026
Contributor
Author
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 1 | gemini | REQUEST_CHANGES
ファイル名補完のロジックについて、結合度の高い実装とテストアサーションの改善点を指摘しました。
ご確認のほどよろしくお願いします。
takemi-ohama
commented
May 24, 2026
Contributor
Author
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 1 | gemini | REQUEST_CHANGES
PLAN03-2 のファイル名自動補完、ご対応ありがとうございます。
S3 URI とローカルパスの双方で末尾 / をディレクトリとして扱う修正、およびユニットテストの追加を確認しました。
1点だけ、可読性向上のための軽微な修正提案をインラインコメントに記載しましたので、ご確認いただけますと幸いです。
- `_generate_default_filename` を新設し `_default_dest` / `_default_filename` の結合を解消 (major / 設計) - `_complete_dir_dest` の `endswith` をタプル形式に統一 (minor / 可読性) - `test_complete_dir_dest_local_trailing_slash` を `os.sep` 完全パス比較 (`==`) に変更 (minor / テスト)
takemi-ohama
commented
May 24, 2026
Contributor
Author
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 2 | codex | APPROVE
重大な修正提案はありません。
takemi-ohama
commented
May 24, 2026
Contributor
Author
takemi-ohama
left a comment
There was a problem hiding this comment.
🤖 cross-review | round 2 | gemini | APPROVE
今回の変更内容は、コードの結合度を下げ、テストの堅牢性を高めるものであり、全体として良い改善だと評価します。
1点、将来のメンテナンス性を考慮し、Docstring 内の PR 番号への言及を削除することを提案します。
| """既定ファイル名 (prefix なし) を生成する共通ヘルパー。 | ||
| `_default_dest` / `_default_filename` の両方から呼ぶことで結合度を下げる | ||
| (PR #25 cross-review round 1 gemini 指摘)。""" | ||
| # microsecond まで含めて衝突を回避する (PR #22 codex round 3 指摘) |
Contributor
Author
There was a problem hiding this comment.
[minor / 可読性] Docstring 内の PR 番号やレビューラウンドへの言及は、将来的なメンテナンス性を考慮すると削除する方が望ましいです。コードやコメントは、それ自体で完結して理解できるように記述されているのが理想です。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
devbase env export s3://bucket/prefix/のように 末尾/の dest を指定したとき、ファイル名部分が空のオブジェクトが作られていた問題を修正する (#24)。aws s3 cpの慣習に合わせて、dest が「ディレクトリ的」な場合は既定ファイル名 (devbase-env-<TS>.dbenv/.dbenv.tar.gz) を自動補完する。関連 Issue
変更点
lib/devbase/env/io_export.py_default_filename()/_complete_dir_dest()を追加// ローカル既存ディレクトリ / 末尾/) なら既定ファイル名を append するtests/cli/test_env_export.py_complete_dir_destの単体テスト 7 件 (S3 末尾/補完 / S3 フルキー回帰 / ローカル既存ディレクトリ補完 / ローカル末尾/補完 / 通常ファイル非補完 / stdio 非補完 / 平文.dbenv.tar.gzサフィックス)動作確認
devbase env export s3://.../prefix/で末尾/補完を実機確認 (任意)補足
issue 「想定される対応」の案 1 (ファイル名自動補完,
aws s3 cp互換) を採用。案 2 (明示エラー) ではなく案 1 を採った理由は plan ファイル
issues/PLAN03-2.mdに記載。