Replies: 9 comments 33 replies
-
|
Not sure if this is made by design that only changes won't upload, but deletions will in |
Beta Was this translation helpful? Give feedback.
-
|
Thats potentially a bug .. local changes (add, modify, delete) should be 100% ignored when using --download-only .. will need to check |
Beta Was this translation helpful? Give feedback.
-
|
I want to keep local version if both sides are being modified in If this is also confirmed as a bug, I would suggest a rewrite to merge all use cases into the same pipeline so that such potential bugs and unexpected behaviors won't occur. It can also reduce duplicate codes that handle similar tasks for future maintenance and scalability, or can further greatly reduce synchronization time for some cases(e.g. local first). I'm wondering if this sounds feasible to be implemented or acceptable to be merged into this version. |
Beta Was this translation helpful? Give feedback.
-
There is no functionality issue here - as previously explained to you I missed adding something back in which is why you were seeing something that should not be occurring in the first place .. ie .. application bug Stop using Use Case:
Result:
This is fixed in |
Beta Was this translation helpful? Give feedback.
-
This is a bug - it is an oversight. I will fix this tomorrow.
This should be an incompatability. You should not be able to use
|
Beta Was this translation helpful? Give feedback.
-
|
@JC-comp |
Beta Was this translation helpful? Give feedback.
-
|
Here is my suggestion for the flags and modes to be used, each should be considered seperately step by step
If we design it this way, there are no restrictions on the use of any combination of the options in theses three steps. So all use cases can be covered.
As far as I understand, OneDrive API is designed only to serve the state of a remote drive, deciding which side is the source of truth is client's job, you can choose to pick up remote version or local version on your will. Follow the above idea, step 1 and 3 is trivial, for step 2, I think this can be achieved by storing the synchronization status of both sides so that we know if a change is a conflict or update from one side.
That's actually not the result I want. What I want is the local file should be the 'source of truth', so that my local changes won't be moved to other places or gone, and I also don't need these modifications to be pushed back online. Then I can use |
Beta Was this translation helpful? Give feedback.
-
The application already does all this operational mode checking. You came across a bug which has since been fixed - and this bug appears to have shifted your understanding on how the application operates.
Not true - the OneDrive API makes the assumption that 100% everything that is stored online, is being stored locally - ie - no client side filtering going on as to what the client would like to store. That this client performs 'client side filtering' breaks the Microsoft assumptions that:
This is then your misunderstanding on how the application operates. The function That you are then modifying this local data, then expecting this to be retained, over and above what the function of As articulated above however, the following now will happen in this use case: This is applicable from |
Beta Was this translation helpful? Give feedback.
-
|
@abraunegg Regarding #2359: I am not able to write anything to my report, because it's locked and limited to collaborators. I hope my feedback reaches you here. I have tested the alpha3 build and can verify that the quota works. thanks |
Beta Was this translation helpful? Give feedback.
-
My apologies - I should have unlocked it. Will do that now. In terms of the feature - is this as per what you were looking for? |
Beta Was this translation helpful? Give feedback.
-
|
Failed case: in local first mode, after synchronization is done, modify local file and move remote file at the same time. Expected result: upload new local version outside folder, keep old remote version inside folder Actual result: log |
Beta Was this translation helpful? Give feedback.
-
|
This is not a failed result .... and your example of what was moved where or what changed locally & online is really unclear You are using Your online move, is moving the exact same object id to a new location. So to the OneDrive API, when the upload file function is called, it is using the prior known location of the file (which you moved) which is why you are ending up with this sort of result. Please be more clearer with your use case. |
Beta Was this translation helpful? Give feedback.
-
|
This is the initial sync state changes:
Result: After one synchronization is completed, the results on both sides are inconsistent. And the old moved version will be gone locally and remotely eventually. I know exactly what is going on there, just to make sure this is consistent with the logic of the design (not a bug). Also consider a remote deletion, local changes will not be uploaded first because remote file is gone. And the following deletion event deleted the local item. So all files are gone.
Result: |
Beta Was this translation helpful? Give feedback.
-
|
This seems to be an existing issue from previous version
|
Beta Was this translation helpful? Give feedback.
-
Potentially - needs to be looked at and investigated |
Beta Was this translation helpful? Give feedback.
-
|
This is a good finding. I think there are two issues: 1) webhook server is not shutdown in a normal exit, and 2) webhook server is not shutdown on SIGINT/SIGTERM (e.g. ^C). I can look into fixing both within this week. Issue 1 was never considered when I first implemented webhooks, however issue 2 was considered and implemented in the original PR #1718. It seems the code has been lost in the 2.5 branch. See Lines 2074 to 2093 in 4a60654 Note the call to Lines 493 to 497 in 4a60654 |
Beta Was this translation helpful? Give feedback.
-
As the documentation says, sqlite does not support multi-thread shared connections. It might lead to undefined behaviour. |
Beta Was this translation helpful? Give feedback.
-
|
Copying comments over from #2520:
From @abraunegg:
This is not specific to containerised environments, right? It happens as long as someone specifies |
Beta Was this translation helpful? Give feedback.
-
It is actually. In 'normal' application use, the default directory when not using a 'config' file is If you change this via adding If you change this via adding In a containerised environment, we are force passing So what needs to occur is there potentially needs to be some additional checks here for this option:
That v2.4.x and below was not correctly triggering when 'sync_dir' was being modified from the default, that is a hidden bug that is being fixed in v2.5.x ... and it brings with it this problem. |
Beta Was this translation helpful? Give feedback.
-
|
Suppose a user runs onedrive outside of a docker container, but uses a shell script to start onedrive. The shell script passes in I am not familiar with the full config handling logic, but it feels to me that any options passed via command-line arguments should override the ones from the config file, and should be saved in the config backup. That way, changes to command-line arguments can also be detected when comparing the current config to the backup config. |
Beta Was this translation helpful? Give feedback.
-
|
@Lyncredible The root cause here, as per above is that in v2.4.x and below was not correctly triggering when 'sync_dir' was being modified from the default, that is a hidden bug that is being fixed in v2.5.x ... and it brings with it this problem. So there are 2 ways to solve for this:
The whole point of testing the 'alpha series of code' is to flush out issues like this long before a beta or release candidate or release is done. |
Beta Was this translation helpful? Give feedback.
-
|
I agree removing I think the issue is a more generic one though. If I understand the code correctly, the program allows overriding any config option via command-line arguments. So I think the config change check logic should take that into account. In my opinion, both the config file and all command-line arguments should be backed up. That way, upon a subsequent startup, the program can examine if any of the config entries or any of the command-line arguments has changed. If any change is detected, it can then decide if a resync is necessary based on which config entry has changed. In a word, I don't think there should be a special case for |
Beta Was this translation helpful? Give feedback.
-
|
To illustrate the problem, imagine a scenario like this:
Throughout the 2 runs, the config file does not change. Should the second run require a resync?
|
Beta Was this translation helpful? Give feedback.
-
When When I read https://www.sqlite.org/compile.html#threadsafe :
SQLITE_CONFIG_SERIALIZED states, as per https://www.sqlite.org/c3ref/c_config_covering_index_scan.html#sqliteconfigserialized
The database object is being shared (opened once, with the object being used / reused throughout) for access to the database. My reading and understanding is there is not going to be a problem here with the way that this 'alpha' series code performs parallel uploads | downloads to speed up the sync process when uploading|downloading numerous files files becomes a far more streamlined operation. The whole catalyst for v2.5.x is to resolve #232 and (primarily) this issue only. |
Beta Was this translation helpful? Give feedback.
-
|
Then this shouldn't be a big issue, seems that most pre-built versions use SQLITE_THREADSAFE = 1 as default option. |
Beta Was this translation helpful? Give feedback.
-
Please can you test the latest PR version as there was a missing API shutdown call which was missing in the exit functions - it was there, but got lost with other work: 6bb380c The PR version should be: v2.5.0-alpha-3 GitHub version: v2.4.25-31-g6bb380c
I tried my best to pull in your changes from #2516 to #2520 however if I have missed something, please can you look at what needs to change. |
Beta Was this translation helpful? Give feedback.
-
|
@abraunegg This commit should fix the issue. You can also apply the following diff. Show diff |
Beta Was this translation helpful? Give feedback.
-
|
Many thanks - I have merged that into 'alpha-3' - so you should be able to test this against PR #2520 now which will yield |
Beta Was this translation helpful? Give feedback.
-
|
@abraunegg Thanks for importing the change. I have been running this version for the last 10 hours. So far so good. |
Beta Was this translation helpful? Give feedback.
-
|
Could you re-test with the latest version of the PR - There was a memory leak in the webhook code which I have fixed - if you can test that would be greatly appreciated. |
Beta Was this translation helpful? Give feedback.
-
|
@abraunegg Tested with both normal exit (using |
Beta Was this translation helpful? Give feedback.
-
|
@Lyncredible This has some additional logic for detecting a Docker container + first run, and saves the sync_dir status if no 'config' file is found, so that it is re-used. First RunSubsequent Run |
Beta Was this translation helpful? Give feedback.
-
|
Thanks for the fix! |
Beta Was this translation helpful? Give feedback.
-
As part of the ongoing development of v2.5.0, client version v2.5.0-alpha-3 is now available for testing.
Changelog from alpha-2:
What this means is that 'alpha-3' now has feature parity with v2.4.x application capability. This is a significant milestone.
Whats still missing?
You can use your normal account if you wish (this is your choice), however this is 100% alpha quality software at the moment. If you experience data loss or something else with your real data - whilst not good, please take every effort to not use this code with your real data.
There will be functionality differences, there will be bugs or things that are not working right.
My ask is that you create a test report - what works, what does not work, what could be potentially changed | improved - basically I am soliciting some feedback on where this total re-write is heading as it matures to a v2.5.0 release.
How can you get v2.5.0-alpha-3 ?
You need to build this client from source. Follow the high level directions here to get the development requirements installed first: https://github.com/abraunegg/onedrive/blob/master/docs/INSTALL.md#building-from-source---high-level-requirements
Next clone and checkout the PR:
To run the PR, you need to run the client from the PR build directory:
When running the PR, your version should be as per below or greater:
The important point is
v2.5.0-alpha-3. Thev2.4.25-XX-abcdefgwill change as new commits are put into the PR branch, whilst things are added/fixed/updated.What is known to be not functioning right at this stage:
alpha-3but will need to be focused upon for future releases once feature parity with v2.4.25 is reached. Please do not raise an issue ticket because of this.I have no test files or test data - what can I use?
In developing this application I use a script as per below to generate random data:
Update
DESTDIRwith the right directory and adjustFILESandFOLDERSas desired. The above settings will create 10 folders, with each folder having 10 files. with each file size between 6KB and 125KB.To increase (or decrease) the random file size, change
BLOCKSIZE, generally use ^2 values: 1,2,4,8,16,32,64,128,256 and so on.At 128 you will get files from 100KB -> ~3.8MB, at 256 most files will be between 500KB --> 7.5MB
Beta Was this translation helpful? Give feedback.
All reactions