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

Improvement: Add time string to filename #19

Merged
merged 8 commits into from
Jun 28, 2019

Conversation

skomma
Copy link
Member

@skomma skomma commented Jun 26, 2019

No description provided.

@skomma skomma force-pushed the imprv/add-time-to-filename branch from 4d42c6f to 4433f5e Compare June 26, 2019 02:23
@skomma skomma force-pushed the imprv/add-time-to-filename branch from 4433f5e to 9398c1e Compare June 27, 2019 02:11
@@ -152,7 +152,7 @@ TODAY=`/bin/date +%Y%m%d` # It is used to generate file name to restore
echo "=== $0 started at `/bin/date "+%Y/%m/%d %H:%M:%S"` ==="

# Validate environment variables
REQUIRED_ENVS=("GCP_ACCESS_KEY_ID" "GCP_SECRET_ACCESS_KEY" "GCP_PROJECT_ID" "TARGET_BUCKET_URL" "DOT_BOTO_OAUTH")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCP_ACCESS_KEY_IDGCP_SECRET_ACCESS_KEY があれば、必ずしも必要には見えなかったので削った。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCS 用のテストでは GCP_ACCESS_KEY_ID と GCP_SECRET_ACCESS_KEY を使って mab を実行するケースと、
DOT_BOTO_OAUTH で指定された OAuth 設定ファイルを使って mab を実行するケースの 2 パターンをテストするので、 DOT_BOTO_OAUTH も必須項目です。(下記で使っている)

echo -e "$DOT_BOTO_OAUTH" > 'conf/.boto_oauth'

Copy link
Member Author

@skomma skomma Jun 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここに書いてることと矛盾しない?

https://github.com/weseek/mongodb-awesome-backup/blob/54b7cd544f1d8d647e7bc4c7c71864131ba4922f/CONTRIBUTING.md#test-for-gcs-only

この手順の通りだと、 DOT_BOTO_OAUTH なんて環境変数は指定しないでテストを実行するように言ってるし、実際 test/gcs/e2e.shREQUIRED_ENVS から削ったうえで、 conf/.boto_oauth が存在していれば DOT_BOTO_OAUTH 環境変数はなくても動いた。
(= 必ずしもテストを動かすのには必須じゃないと思ってる)

CircleCI で環境変数指定で渡すために使ってるとか?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かに手順と矛盾しますね。
必須ではなく、「config/.boto_oauth ファイルを作成するか、環境変数 DOT_BOTO_OAUTH を指定する必要がある」が正しいですね。
そうなると必須ではないので除外すべきだと思います。 👍

test/gcs/entrypoint.init_test.sh Show resolved Hide resolved
test/s3/e2e.sh Show resolved Hide resolved
test/s3/e2e.sh Show resolved Hide resolved
bin/backup.sh Show resolved Hide resolved
bin/functions.sh Outdated
create_current_yyyymmddhhmmss() {
echo `/bin/date +%Y%m%d%H%M%S`
}

# Create today's date string(YYYYmmdd)
create_today_yyyymmdd() {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

テスト時に使うので、today は引き続き取っておいてある。

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

使ってませんでした orz
機会があれば test で function.sh を取り込んで create_today_yyyymmdd を使うように修正ですかね。

TODAY=`/bin/date +%Y%m%d` # It is used to generate file name to restore

TODAY=`/bin/date +%Y%m%d` # It is used to generate file name to restore

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ほんまや。

機会があれば test で function.sh を取り込んで create_today_yyyymmdd を使うように修正ですかね。

テストが本体側のコードを参照するのはなんかおかしい気もするので、いったん消した。

test/gcs/e2e.sh Show resolved Hide resolved
test/gcs/Dockerfile.init_test Show resolved Hide resolved
@skomma skomma marked this pull request as ready for review June 27, 2019 05:16
@skomma skomma changed the title Add time to filename Improvement: Add time string to filename Jun 27, 2019
bin/functions.sh Outdated
create_current_yyyymmddhhmmss() {
echo `/bin/date +%Y%m%d%H%M%S`
}

# Create today's date string(YYYYmmdd)
create_today_yyyymmdd() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

使ってませんでした orz
機会があれば test で function.sh を取り込んで create_today_yyyymmdd を使うように修正ですかね。

TODAY=`/bin/date +%Y%m%d` # It is used to generate file name to restore

TODAY=`/bin/date +%Y%m%d` # It is used to generate file name to restore

@@ -152,7 +152,7 @@ TODAY=`/bin/date +%Y%m%d` # It is used to generate file name to restore
echo "=== $0 started at `/bin/date "+%Y/%m/%d %H:%M:%S"` ==="

# Validate environment variables
REQUIRED_ENVS=("GCP_ACCESS_KEY_ID" "GCP_SECRET_ACCESS_KEY" "GCP_PROJECT_ID" "TARGET_BUCKET_URL" "DOT_BOTO_OAUTH")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCS 用のテストでは GCP_ACCESS_KEY_ID と GCP_SECRET_ACCESS_KEY を使って mab を実行するケースと、
DOT_BOTO_OAUTH で指定された OAuth 設定ファイルを使って mab を実行するケースの 2 パターンをテストするので、 DOT_BOTO_OAUTH も必須項目です。(下記で使っている)

echo -e "$DOT_BOTO_OAUTH" > 'conf/.boto_oauth'

@@ -152,7 +152,7 @@ TODAY=`/bin/date +%Y%m%d` # It is used to generate file name to restore
echo "=== $0 started at `/bin/date "+%Y/%m/%d %H:%M:%S"` ==="

# Validate environment variables
REQUIRED_ENVS=("GCP_ACCESS_KEY_ID" "GCP_SECRET_ACCESS_KEY" "GCP_PROJECT_ID" "TARGET_BUCKET_URL" "DOT_BOTO_OAUTH")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

確かに手順と矛盾しますね。
必須ではなく、「config/.boto_oauth ファイルを作成するか、環境変数 DOT_BOTO_OAUTH を指定する必要がある」が正しいですね。
そうなると必須ではないので除外すべきだと思います。 👍

@ryu-sato ryu-sato merged commit f036b1a into master Jun 28, 2019
@ryu-sato ryu-sato deleted the imprv/add-time-to-filename branch June 28, 2019 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants