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

onedrive client version v2.5.0-alpha-3 #2520

Closed
wants to merge 42 commits into from
Closed

Conversation

abraunegg
Copy link
Owner

  • Development tree for onedrive client version v2.5.0-alpha-3

commit 1eff2d7
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Wed Oct 18 19:15:27 2023 +1100

    Update PR

    * Add  --source-directory 'path/as/source/' --destination-directory 'path/as/destination' functionality

commit ad3ddee
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Wed Oct 18 17:32:24 2023 +1100

    Update PR

    * Add --create-directory
    * Add --remove-directory

commit 7dfe6b6
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Wed Oct 18 12:27:03 2023 +1100

    Update PR

    * Update PR

commit 75c071e
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Wed Oct 18 10:12:05 2023 +1100

    Update PR

    * Update PR

commit 6db484c
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Mon Oct 16 17:01:25 2023 +1100

    Update PR

    * Update PR

commit d893ea5
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Wed Oct 11 10:43:50 2023 +1100

    Update PR

    * Update PR

commit 82bd593
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Wed Oct 11 09:14:17 2023 +1100

    Update PR

    * Validate and document --auth-files operation

commit c551203
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Wed Oct 11 05:48:22 2023 +1100

    Update PR

    * Add --create-share-link

commit fbf6399
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Tue Oct 10 18:39:21 2023 +1100

    Update PR

    * Update PR

commit 72a4680
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Tue Oct 10 17:43:15 2023 +1100

    Update PR

    * Add --get-file-link
    * Add --modified-by

commit 0d3fc3e
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Tue Oct 10 14:28:10 2023 +1100

    Add --display-sync-status

    * Add --display-sync-status

commit 1f183ca
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Mon Oct 9 08:18:13 2023 +1100

    Update PR

    * Update PR with doc updates

commit b0628d7
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Sun Oct 8 10:52:52 2023 +1100

    Update PR

    * Update PR

commit 7e3df95
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Sat Oct 7 05:31:26 2023 +1100

    Update PR

    * Update PR

commit c69f2ab
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Sat Oct 7 05:28:28 2023 +1100

    Update PR

    * Update PR

commit ea1ca33
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Fri Oct 6 14:57:51 2023 +1100

    Update PR

    * Update PR

commit 1503f96
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Fri Oct 6 09:19:04 2023 +1100

    Update PR

    * Update PR

commit 5127464
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Fri Oct 6 06:48:20 2023 +1100

    Change when the integrity check is performed

    * Change when the integrity check is performed

commit c7cc45d
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Thu Oct 5 19:40:05 2023 +1100

    Update maxInotifyWatches location

    * Update maxInotifyWatches location

commit c44ad96
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Thu Oct 5 17:41:31 2023 +1100

    Update main.d

    * Fix --version segfault

commit 51f0ffc
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Thu Oct 5 17:24:30 2023 +1100

    Uplift to v2.5.0-alpha-2

    * Uplift to v2.5.0-alpha-2

commit cbe3e6e
Author: abraunegg <alex.braunegg@gmail.com>
Date:   Thu Oct 5 17:17:26 2023 +1100

    Clean up before onedrive-v2.5.0-alpha-2

    * Clean up before onedrive-v2.5.0-alpha-2
* Uplift to v2.5.0-alpha-3
@abraunegg abraunegg added this to the v2.5.0 milestone Oct 18, 2023
* Add webhook functionality back in
@abraunegg
Copy link
Owner Author

abraunegg commented Oct 18, 2023

@Lyncredible

Please can you test this 'alpha-3' version?

This is an uplift of your original code (as per v2.4.25) and does not include any of your changes as per #2516

If you can confirm that this current uplift works, could you then please uplift this PR with your changes from #2516 ?

This also includes your changes from #2516 - the code compiles, but I am unable to validate and check.

If you could so some sanity checking that would be greatly appreciated.

* Fix that the '.' were not being printed in --verbose mode for fetching the /delta response
* Update webhook feature with #2516 changes
* Add documentation notes for webhooks
* Fix variable naming
* --download-only and --local-first cannot be used together
* Fix onedrive -s --download-only
* Fix onedrive -s --download-only --local-first
@Lyncredible
Copy link
Contributor

@abraunegg I had to resync but it seems to be working. I will report back after the resync is done.

See in-progress logs
Base Args: --monitor
# Launching onedrive
Reading configuration file: /onedrive/conf/config
Configuration file successfully loaded

The usage of --resync will delete your local 'onedrive' client state, thus no record of your current 'sync status' will exist.
This has the potential to overwrite local versions of files with perhaps older versions of documents downloaded from OneDrive, resulting in local data loss.
If in doubt, backup your local data before using --resync

Are you sure you wish to proceed with --resync? [Y/N] y

The item database is incompatible, re-creating database table structures
Deleting the saved application sync status ...
Configuring Global Azure AD Endpoints
Sync Engine Initialised with new Onedrive API instance
OneDrive synchronisation interval (seconds): 86400
Initialising filesystem inotify monitoring ...
Performing initial syncronisation to ensure consistent local state ...
Initializing subscription for updates ...
Webhook: handled validation request
Created new subscription <REDACTED> with expiration: <REDACTED>
Starting a sync with Microsoft OneDrive
Fetching items from the OneDrive API for Drive ID: <REDACTED> ..................

@Lyncredible
Copy link
Contributor

Lyncredible commented Oct 19, 2023

@abraunegg Resync is done. Webhook is working, but it seems something is wrong with the resync detection logic. It asks me to do a resync again when I restarted it without --resync. I can debug further tomorrow.

Log for successful resync
Base Args: --monitor
# Launching onedrive
Reading configuration file: /onedrive/conf/config
Configuration file successfully loaded

The usage of --resync will delete your local 'onedrive' client state, thus no record of your current 'sync status' will exist.
This has the potential to overwrite local versions of files with perhaps older versions of documents downloaded from OneDrive, resulting in local data loss.
If in doubt, backup your local data before using --resync

Are you sure you wish to proceed with --resync? [Y/N] y

The item database is incompatible, re-creating database table structures
Deleting the saved application sync status ...
Configuring Global Azure AD Endpoints
Sync Engine Initialised with new Onedrive API instance
OneDrive synchronisation interval (seconds): 86400
Initialising filesystem inotify monitoring ...
Performing initial syncronisation to ensure consistent local state ...
Initializing subscription for updates ...
Webhook: handled validation request
Created new subscription <REDACTED> with expiration: <REDACTED>
Starting a sync with Microsoft OneDrive
Fetching items from the OneDrive API for Drive ID: <REDACTED> ..........................................................................................................................................................................................................................................................
Processing <REDACTED> applicable changes and items received from Microsoft OneDrive ...........................................................................................................
Performing a database consistency and integrity check on locally stored data ...
Scanning the local file system '/onedrive/data' for new data to upload ...
Performing a last examination of the most recent online data within Microsoft OneDrive to complete the reconciliation process
Fetching items from the OneDrive API for Drive ID: <REDACTED> ..
No additional changes or items that can be applied were discovered while processing the data received from Microsoft OneDrive
Sync with Microsoft OneDrive is complete
Initializing subscription for updates ...
Webhook: handled validation request
Created new subscription <REDACTED> with expiration: <REDACTED>
^CGot termination signal, performing clean up
Shutting down DB connection and merging temporary data
Log for a subsequent run without `--resync`
usermod: no changes
Base Args: --monitor
# Launching onedrive
Reading configuration file: /onedrive/conf/config
Configuration file successfully loaded

An application configuration change has been detected where a --resync is required

* Add a check when using --download-only to test if the file already exists locally, and if it does, has the local file been modified since the file was last downloaded. If it has, then rename the existing file to preserve it to prevent local data loss
@abraunegg
Copy link
Owner Author

abraunegg commented Oct 19, 2023

@Lyncredible

Log for a subsequent run without --resync

Can you run the application in debug --verbose --verbose mode ??

Need to understand what is triggering the --resync here post reading in the configuration file .. something is changing ... need to understand what it is and why the application thinks it has changed.

Also all this logic is 100% different to v2.4.x series - so if you ran v2.4.x inbetween ... could be an indicator as to where the issue is occurring from.

* Implement --display-quota
@Lyncredible
Copy link
Contributor

@abraunegg
Resync was deemed necessary because it detected sync_dir override from CLI:

# The log statement was changed to also log the two different values
[DEBUG] sync_dir: CLI override of config file option, --resync needed, config = ~/OneDrive, CLI = /onedrive/data

The configured value is hard-coded since I never specified sync_dir in my own config file:

immutable string defaultSyncDir = "~/OneDrive";

The CLI override comes from the entrypoint.sh in the default docker image:

ARGS=(--monitor --confdir /onedrive/conf --syncdir /onedrive/data)

I now have specified sync_dir in my own config file to match the CLI override as a workaround, but this requires another resync since the config file has changed...

@Lyncredible
Copy link
Contributor

@abraunegg
I hit a new issue where access token renewal fails:

AADSTS9000410: Malformed JSON.
ERROR: You will need to issue a --reauth and re-authorise this client to obtain a fresh auth token.

This is because the request to renew token is sent with the Content-Type: application/json header, but the token endpoint requires Content-Type: application/x-www-form-urlencoded. The wrong header is sent because http.clearRequestHeaders() is no longer called in scope(exit) {} in OneDriveApi.post() (or other methods like OneDriveApi.patch().

For example, I see http.clearRequestHeaders() being called in master branch:

onedrive/src/onedrive.d

Lines 1392 to 1401 in 4a60654

private auto post(T)(const(char)[] url, const(T)[] postData)
{
scope(exit) http.clearRequestHeaders();
http.method = HTTP.Method.post;
http.url = url;
addAccessTokenHeader();
auto response = perform(postData);
checkHttpCode(response);
return response;
}

But it is not found in onedrive-v2.5.0-alpha-3:

onedrive/src/onedrive.d

Lines 1427 to 1434 in 77c294b

private auto post(T)(string url, const(T)[] postData) {
curlEngine.setMethodPost();
curlEngine.http.url = url;
addAccessTokenHeader();
auto response = perform(postData);
checkHttpResponseCode(response);
return response;
}

Why was the line removed?

@abraunegg
Copy link
Owner Author

@Lyncredible

For example, I see http.clearRequestHeaders() being called in master branch

OK - thats a gap & bug in 'alpha-3' that needs to be corrected. Will look at that shortly.

Also - just to keep comments / feedback in the same place, could you provide feedback here: #2521 rather than in this PR that would be great.

@abraunegg
Copy link
Owner Author

@Lyncredible

Resync was deemed necessary because it detected sync_dir override from CLI:

OK .. thanks for this feedback .. essentially as 'sync_dir' is now being correctly being picked up as a change, this is what is happening. I am going to have to look at this a little closer on how to work around this for containerised environments.

* Fix memory leak on exit when using webhooks
* When --syncdir is being used, and no initial config file exists, write the --syncdir value to a new default configuration file, so that when the client is run again, the --syncdir value is used. If an existing config file is present, update the sync_dir config value that was passed in, but only if it is different to the existing set value
* Update PR to add a test for entrypoint.sh to exist, to assist with Docker first run scenario
* Update Podman and Docker documentation
* Update doc
* Update doc
* Update doc
* Add --dry-run parameter for Docker and Podman as per #2536

Authored by: @mbrouwer
* Add LMDE 5, LMDE 6 and Ubuntu 23.10
* Finally replace safeRename() with safeBackup() to ensure consistent messaging and process when a local file is being renamed.
* Check the system for any session files, that indicate that a session upload did not complete
* Change from using a CRC32 of the file, to a random 16 character alphanumeric string to use as the session filename extension as it is not computationally valid to do a CRC32 on large files
…ption

* Handle --upload-only & --remove-source-files for upload session resumption
* Add Ubuntu 23.10 details
* Correct product name
* Potentially fix crash identified by #2562
Change the handling of selfBuiltPath so correct path is used when attempting to test against 'sync_list'
* Add warning message about HTML characters in JSON element
* Update logging output
@abraunegg abraunegg closed this Dec 21, 2023
@abraunegg
Copy link
Owner Author

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Request: Add cmdline parameter to display (human readable) quoa status
3 participants