Add the tempDir as parameter #3065
Conversation
ghost
requested review from
crawter and
mniewrzal
and removed request for
a team
|
@TopperDEL thanks for contribution, to make this change complete you need also update automatic tests https://build.dev.storj.io/blue/organizations/jenkins/storj/detail/PR-3065/1/tests |
|
I've corrected the automated test, but it failed again. To be honest, I don't know what went wrong. My Change should not Impact what I see in the test-fail-log. Can someone have a look at it and tell me if I Need to adjust my PR any further? |
|
@bryanchriswhite can you take a look and give some help to @TopperDEL ? |
|
👀 |
|
@TopperDEL try merging https://github.com/TopperDEL/storj/pull/2 and see if that fixes jenkins. |
rearrange args & use real tempdir in c tests
|
I'm still looking into this. I'm not sure yet of the root cause of the backwards-compatibility check failure. |
bryanchriswhite
added
Debug
There was a problem hiding this comment.
I am unable to reproduce this error locally. When I run:
psql -U postgres -c 'create database teststorj3;' && STORJ_SIM_POSTGRES=postgres://postgres@localhost/teststorj3?sslmode=disable make test-sim-backwards-compatible
I don't see the Bucket not empty: bucket-123 error that shows up at the end of jenkins' backwards-compatibility check log.
I opened a PR with a branch on the storj repo (#3148) for debugging jenkins.
|
I'm sorry that I cannot help you out on this. |
|
@TopperDEL @bryanchriswhite it looks that failing test was some kind of instability around master. Now its passing. |
Hooray! |
bryanchriswhite
added
Code Review In-Progress
| @@ -23,8 +23,8 @@ type Uplink struct { | |||
| // an error in cerr, when there is one. | |||
| // | |||
| // Caller must call close_uplink to close associated resources. | |||
| func new_uplink(cfg C.UplinkConfig, cerr **C.char) C.UplinkRef { | |||
| scope := rootScope("") // TODO: pass in as argument | |||
| func new_uplink(cfg C.UplinkConfig, tempDir *C.char, cerr **C.char) C.UplinkRef { | |||
There was a problem hiding this comment.
I'm just wondering if we should make tempDir part of UplinkConfig to avoid keeping it as a separate parameter.
There was a problem hiding this comment.
My hesitation with that is that besides breaking symmetry with the go struct, the uplink config is still not well defined as it currently contains a single Volatile struct. Additionally, keeping it as a parameter makes it more visible and clear that it's required (which apparently it is).
What:
When creating a new Uplink I wanted to provide a temp-directory to use - this is necessary at least on Android.
Why:
Otherwise Uploads (and presumably Downloads) on Android won't work.
Please describe the tests:
Tested it with my .Net-Wrapper (uplink.NET).
Please describe the performance impact:
Should be None.
Code Review Checklist (to be filled out by reviewer)