Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

cmd: don't use ':' in the default log file name #236

Merged
merged 1 commit into from
Apr 16, 2020

Conversation

kennytm
Copy link
Collaborator

@kennytm kennytm commented Apr 16, 2020

What problem does this PR solve?

: is not a valid filename character for NTFS. The default log file path contains a : and thus invalid.

What is changed and how it works?

Replace the : in the filename by ..

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    • run bin/br --help and check the output.

Code changes

Side effects

Related changes

@codecov
Copy link

codecov bot commented Apr 16, 2020

Codecov Report

Merging #236 into master will decrease coverage by 0.31%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #236      +/-   ##
==========================================
- Coverage   70.67%   70.36%   -0.32%     
==========================================
  Files          48       48              
  Lines        4744     4744              
==========================================
- Hits         3353     3338      -15     
- Misses        950      965      +15     
  Partials      441      441              
Impacted Files Coverage Δ
cmd/cmd.go 66.17% <100.00%> (ø)
pkg/backup/client.go 71.21% <0.00%> (-2.85%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b37ae8f...e82b0b1. Read the comment docs.

@YuJuncen
Copy link
Collaborator

LGTM

@YuJuncen YuJuncen added the status/LGT1 LGTM1 label Apr 16, 2020
@kennytm kennytm merged commit f5c37f3 into master Apr 16, 2020
@kennytm kennytm deleted the kennytm/no-colon-in-default-log-path branch April 16, 2020 10:19
@kennytm kennytm added status/LGT2 LGTM2 and removed status/LGT1 LGTM1 labels Apr 16, 2020
3pointer pushed a commit to 3pointer/br that referenced this pull request Apr 26, 2020
kennytm added a commit that referenced this pull request Apr 26, 2020
* enable tidb config by default (#230)

* enable tidb config by default

* backup,restore: fix --checksum flag. (#223)

* backup,restore: work on progress to fix a bug that causes --checksum flag won't work properly.

Some code of backup and restore ignored the flag (a.k.a. config.Checksum), so when checksum is disabled, we will face failure.

* backup: backup will report total bytes and kvs when checksums check disabled.

Some code of backup and restore ignored the flag (a.k.a. config.Checksum), so when checksum is disabled, we will face failure.

* backup: backup will report total bytes and kvs when checksums check disabled.

Some code of backup and restore ignored the flag (a.k.a. config.Checksum), so when checksum is disabled, we will face failure.

* backup: add log to ChecksumMatches and new version of FastChecksum.

Some of log has been lose. They are in ChecksumMatches now.

* restore: restore could find non-checksum tables and skip them automatically.

for backup, ChecksumMatches returns error now.

* misc: add document for Table::NoChecksum.

* backup: omit checksum progress bar when user specify `--checksum=false`.

* backup: `CopyMetaFrom` overrides original `client.Schemes` instead of append at its end.

* backup: refactor about checksum logic, fix a bug.

the bug would cause: when multi tables are backup, the metadata contains only one table.

* backup: do some lints.

* backup,restore: do some refactor so that cyclomatic complexity won't be too large.

* misc: don't use underscore on receiver.

* backup: print "quick checksum success" message per table.

...to make br_full_index happy!

* backup: refactor a MinInt pattern.

* backup: Apply suggestions from code review

Co-Authored-By: kennytm <kennytm@gmail.com>

Co-authored-by: 3pointer <luancheng@pingcap.com>
Co-authored-by: kennytm <kennytm@gmail.com>

* Use table info (#231)

* create with info during incremental restore

* add test

* fix log

* address comment

* fix ci

* address commemnt

* backup: generate backupmeta when backup empty. (#235)

* cmd: don't use ':' in the default log file name (#236)

* Update build status badge (#239)

* pass sse_kms_key_id to S3 (#243)

* redirect kvproto

Signed-off-by: Yi Wu <yiwu@pingcap.com>

* pass sse_kms_key_id

Signed-off-by: Yi Wu <yiwu@pingcap.com>

* fix hound

Signed-off-by: Yi Wu <yiwu@pingcap.com>

* update kvproto

Signed-off-by: Yi Wu <yiwu@pingcap.com>

* go mod tidy

Signed-off-by: Yi Wu <yiwu@pingcap.com>

Co-authored-by: kennytm <kennytm@gmail.com>

* storage: support placing the S3/GCS options into the storage URL (#246)

* update 4.0 dependency

* go mod tidy

* Update README.md

Co-Authored-By: Neil Shen <overvenus@gmail.com>

Co-authored-by: 山岚 <36239017+YuJuncen@users.noreply.github.com>
Co-authored-by: kennytm <kennytm@gmail.com>
Co-authored-by: Neil Shen <overvenus@gmail.com>
Co-authored-by: yiwu-arbug <yiwu@pingcap.com>
3pointer pushed a commit to 3pointer/br that referenced this pull request Apr 26, 2020
3pointer added a commit that referenced this pull request Apr 26, 2020
* backup: generate backupmeta when backup empty. (#235)

* cmd: don't use ':' in the default log file name (#236)

* Update build status badge (#239)

* tools: replace retool by GO111MODULE (#244)

* tools: replace retool by GO111MODULE

* Makefile: temporarily shut up golangci-lint

* Restore with lower case (#240)

* restore: use lower string to filter since we use lower string to filter backup

Co-authored-by: kennytm <kennytm@gmail.com>

* storage: support placing the S3/GCS options into the storage URL (#246)

* Rebase auto random counter after restore (#248)

* *: support auto random backup & restore

* update release 3.1

Co-authored-by: 山岚 <36239017+YuJuncen@users.noreply.github.com>
Co-authored-by: kennytm <kennytm@gmail.com>
Co-authored-by: Neil Shen <overvenus@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants