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

support for rds #8

Merged
merged 1 commit into from
May 12, 2014
Merged

support for rds #8

merged 1 commit into from
May 12, 2014

Conversation

toritori0318
Copy link
Collaborator

RDS対応を行いました。以下の点を変えています。
・CLIにサブコマンド追加(rikanjo ec2 / rikanjo rds)
・CLIのRDSコマンドのみ--multiazオプション追加
・モードによりec2/rdsクラスを切り分け
・Net::HTTP.getの使い方を変更(既存の手法だと何故かrdsのjsが取れなかったため)

ec2とrdsで「インタフェースは同じ」かつ「中身が異なる」箇所が多々あった関係で
コードを結構変えてしまいました…
問題ありましたらご指摘お願い致します。

@kenjiskywalker
Copy link
Owner

@toritori0318

もし動くならこれで一旦mergeしてしまって問題ないかと思います。

変数に何が入ってるのか、デバッグ目的でJSONが格納されている
URLへアクセスしようと思っても、色々と処理が入っていて
URLを抽出するのにも結構苦労するのでコメントか何か書いた方がいいかもしれないですね。

ちょっとコードが大きくなってきたので簡潔にできるところは簡潔にできるよう
全体的な見直ししようかと思います。

あとregionを変更すると尽くエラーになるのでテストを書いて
現状何が動くのか確認した方が良いですね。

時間があれば

  • テストの追加
  • コメントの追加
  • 全体的な処理のリファクタリング(簡潔化)

をします。@toritori0318 さんもお時間あればご協力をお願いします!

toritori0318 added a commit that referenced this pull request May 12, 2014
@toritori0318 toritori0318 merged commit fd7156a into master May 12, 2014
@toritori0318 toritori0318 deleted the support_rds branch May 12, 2014 15:18
@toritori0318
Copy link
Collaborator Author

@kenjiskywalker ご確認ありがとうございます!
テストはJSONバリデーションくらいは追加で書こうと思っていたので
書き上げたらmasterブランチに上げておきます

@toritori0318 toritori0318 mentioned this pull request May 12, 2014
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.

None yet

2 participants