フィードバック

feedback.md

もろもろフィードバック

embulk-output-ncmbへのフィードバック

まずgit clone後に./gradlew gemコマンドでビルドして生成されたgemをembulk runしてもParticalExecutionExceptionが発生して動作しないです。 git cloneしただけの状態でembulk runすると動作するので何かgit add漏れがあると思います。

ちなみに開発中は都度gem installをしなくても以下のコマンドで実行可能です。

$ cd /path/to/embulk-output-ncmb
$ embulk run -I lib ~/path/to/ncmb.yml
(中略)
org.embulk.exec.PartialExecutionException: java.lang.RuntimeException: java.lang.NoClassDefFoundError: org/embulk/output/ncmb/PostRequest
(中略)

期待する動作

$ cd /path/to/embulk-output-ncmb
$ ./gradlew gem
# Exceptionが発生せず正常に動作すること
$ embulk run -I lib ~/path/to/ncmb.yml

Embulk関連

• そもそもembulk newコマンドで生成されるひな形をベースにするのがよいと思います。

　そこにNCMB独自のクラスとか足せばいいんではないかと。

• FileOutputプラグインであればFileOutputPluginをimplementsしたクラス(例：embulk-output-gcs)、OutputプラグインであればOutputPluginをimplementsしたクラス(例：embulk-output-elasticsearch)が存在するはずですが、レポジトリ上では見当たりません。

• 前述のようにgit addから漏れているかと。

• プラグインはJRubyでも書けるのでRubyの人口が多いのであればそちらで書くのもありかと。

• filter/parser/formatterはJavaの処理速度の優位性が如実に出る部分なのでJavaの方が良いですが、Input/OutputはJRubyでも行ける場合はあると思います。

Gradle周り

• ニフティクラウドのSDKなど以外はbuild.gradleに含める必要はないと思われます。 生のjarファイルは特に。

• gradlewファイルに対して実行パーミッションが付与されていないのでforkした人がコマンドを叩けない。

• ここはembulk newでプロジェクトを生成した時に存在するものを使えばOK

• Java7ターゲットで提供するなら以下の記述が必要(参考)。Java8ターゲットでビルドするなら不要ですがJava8標準のOptionalなどは使えません。

sourceCompatibility = 1.7
targetCompatibility = 1.7

• gemとしてリリースする際はauthorとかhomepageとかも追記すると良いです。

Javaとかバージョン管理全般に関して

• jarファイルが一緒にcommitされていますが特別な理由がない限り不要。 /jar以下、/lib以下、/classpath/以下等。というよりディレクトリ自体レポジトリにUPする必要はないです。
• 同様にpkg/直下のgemファイルも含めるべきではないです。.gitignoreでpkg/以下をignoreしてるならなおさら。
• buildディレクトリについてもレポジトリに上げる必要はないです。

例外処理

• Exceptionをthrowするのは良くないと思います。明示的にHogeExceptionなどするのが良いです。IntelliJなどのIDE等でプログラミングする際にも該当のメソッドをcallしている箇所の記述が簡単になります。

String doPost(String className, JSONObject postData) throws HogeException, FugaException {
}

try {
    doPost("NCMB", postData);
// IDEの場合、doPostをcallした場合にHogeExceptionとFugaExceptionがthrowされるのでそれを処理するべきということをサジェストしてくれる
} catch (HogeException | FugaException ex) {
    	
}

• 外部のサービスに接続するのであればHTTPのconnectionをopenしている部分などはtry/catch使って例外処理が必要と思います。

• 認証エラー、APIが50xで接続できないetc。参考(Embulkプラグインのリトライ/例外処理の実装について)

• 値が変わりそうなものはEmbulkのConfigで設定できるようにした方が良いと思います。

  • URIの生成ルールが分からないですが一部のパラメータのみ可変(2013-09-01だけ変わるとか)ならそこだけ設定できるようにしてURI全体はプログラム内で組み立てるとか。

• インデントに4tab、4spaceが混在している

その他

OJTでやっているなら書きっ放しだと意味がないので、レビュアー(メンター?)にレビューお願いするのがいいと思います。 Embulk独自の部分はともかくJava周りとかは答えてくれると思いますよ？