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

v1.57 release #5734

Closed
ncw opened this issue Oct 18, 2021 · 22 comments
Closed

v1.57 release #5734

ncw opened this issue Oct 18, 2021 · 22 comments
Milestone

Comments

@ncw
Copy link
Member

ncw commented Oct 18, 2021

Dear @rclone/rclone

The v1.57 release is overdue! I've pushed it back until the end of the month (12 days from now) to give us a little breathing space.

In those days I'd like to

PRs

I'd like a list of PRs that you'd like merged for the release. In particular from @ivandeex and @albertony I have lots of outstanding PRs which need reveiwing so which of those do you want for v1.57. And any other candidates!

Integration tests

As far as the integration tests go there are 35 failing tests which is more than I'm happy with (I've beaten this down from 126 already!), so I'd like some help with those if possible.

  • @buengese have you got time to look at the zoho and compress ones?
  • @ivandeex can you have a look at the chunker ones?
  • There are quite a few TestSyncConcurrentTruncate failures - since you introduced this test @ivandeex would you mind looking at those?
  • I'll look at the azureblob ones and the filefabric ones
  • There is also TestCopyFileMaxTransfer which should fix TestDrive and TestCryptDrive - any takers for that? @albertony ?

That should bring the list down to a more manageable level. I'll just note that the integration tests for mega are often broken as are the ones for qingcloud. The Onedrive china tests are broken because the access token needs refreshing - I'll try to sort that out!

@ncw ncw added this to the v1.57 milestone Oct 18, 2021
@ivandeex
Copy link
Member

ivandeex commented Oct 18, 2021

Pull requests I'm taking part in: https://github.com/rclone/rclone/pulls/assigned/ivandeex

I'd like to see these in 1.57:

Two FTP PRs are waiting for upstream
but can go in 1.57 if we decide on upstream fork (see #5652 (comment)):

I plan to have bisync in 1.58 only, will merge after 1.57 release to start beta testing:

Same plan for resume functions - merge after 1.57 for beta testers, sort out by 1.58:

@ivandeex
Copy link
Member

ivandeex commented Oct 18, 2021

fix integration tests for chunker - #5735

@albertony
Copy link
Contributor

At least these ones (I just marked them with v1.57 milestone, hope that's ok). I don't think they should be too heavy, would be nice to get them on the 🚢

Then we have this tiny little commit here. Nice to have, but it has some risk of regressions we're not able to think of, so I will not decide.. :)

The following two would be nice to get done due to risk of conflicts and additional work, but may take some time to review and finalize, and not important for the release as such:

@creativeprojects
Copy link
Collaborator

There's also this very small one, which is a regression from 1.56

(but not too important, I don't think Seafile is a particularly popular remote 😆 )

@albertony
Copy link
Contributor

  • There is also TestCopyFileMaxTransfer which should fix TestDrive and TestCryptDrive - any takers for that? @albertony ?

I'm looking into it.

@albertony
Copy link
Contributor

  • There is also TestCopyFileMaxTransfer which should fix TestDrive and TestCryptDrive - any takers for that? @albertony ?

I'm looking into it.

The test fails because it expects a copy with MaxTransfer and CutoffModeHard should return Fatal error, because this is thrown from accounting (ErrorMaxTransferLimitReachedFatal), but in case of Google Drive the external google drive API catches and replaces it with a non-fatal error:

pw.CloseWithError(fmt.Errorf("googleapi: Copy failed: %v", err))

(https://github.com/googleapis/google-api-go-client/blob/7290f25351cc90fdeb492127c98cc3fb023793d4/internal/gensupport/media.go#L140)

Don't know how to fix this..

@buengese
Copy link
Member

I've been rather absent from rclone related things for a while, so there are quite a few things I want to catch up on. I'll look into the failing zoho and compress tests first. If I have the time I'm also going to look into the failing encoding tests for 1fichier.

@ncw
Copy link
Member Author

ncw commented Oct 19, 2021

@albertony wrote:

The test fails because it expects a copy with MaxTransfer and CutoffModeHard should return Fatal error, because this is thrown from accounting (ErrorMaxTransferLimitReachedFatal), but in case of Google Drive the external google drive API catches and replaces it with a non-fatal error:

pw.CloseWithError(fmt.Errorf("googleapi: Copy failed: %v", err))

(https://github.com/googleapis/google-api-go-client/blob/7290f25351cc90fdeb492127c98cc3fb023793d4/internal/gensupport/media.go#L140)

Ah, excellent sleuthing.

In a post go1.13 world, changing

pw.CloseWithError(fmt.Errorf("googleapi: Copy failed: %v", err))

to this with a %w specifier would fix the problem in rclone as rclone can unwrap those kinds of errors (I think!).

pw.CloseWithError(fmt.Errorf("googleapi: Copy failed: %w", err))

You could try proposing this to the google-api-go-client repo. Be careful though as nearly all the code is auto generated so you might have to patch the generator! (I've sent a few fixes in the past.)

In the mean time we can mark it as an expected failure in TestCryptDrive and TestDrive in here:

https://github.com/rclone/rclone/blob/master/fstest/test_all/config.yaml

ncw added a commit that referenced this issue Oct 19, 2021
In

05f1288 azureblob: add --azureblob-no-head-object

we incorrectly parsed the size of the object as the Content-Length of
the returned header. This is incorrect in the presense of Range
requests.

This fixes the problem by parsing the Content-Range header if
avaialble to read the correct length from if a Range request was
issued.

See: #5734
ncw added a commit that referenced this issue Oct 20, 2021
Before this change the backoff for the error_background error was 6
seconds. This means that if it wasn't resolved in 60 seconds (with the
default 10 low level retries) then an error was reported.

This error was being reported frequently in the integration tests, so
is likely affecting real users too.

This patch changes the backoff into an exponential backoff
1,2,4,8...1024 seconds to make sure we wait long enough for the
background operation to complete.

See #5734
@ncw
Copy link
Member Author

ncw commented Oct 20, 2021

@buengese wrote:

I've been rather absent from rclone related things for a while

No worries, and nice to see you again :-)

so there are quite a few things I want to catch up on. I'll look into the failing zoho and compress tests first. If I have the time I'm also going to look into the failing encoding tests for 1fichier.

Thank you.

ncw added a commit that referenced this issue Oct 20, 2021
The TestIntegration/FsMkdir/FsPutFiles/PublicLink test doesn't work on
a standard dropbox account, only on an enterprise account because it
sets expiry dates.

See: #5734
ncw added a commit that referenced this issue Oct 20, 2021
The TestIntegration/FsMkdir/FsPutFiles/PublicLink test doesn't work on
a standard onedrive account, it returns

    accessDenied: accountUpgradeRequired: Account Upgrade is required for this operation.

See: #5734
ncw added a commit that referenced this issue Oct 20, 2021
This tries to fix the integration tests by only allowing one
premiumizeme test to run at once, in the hope it will stop rclone
hitting the rate limits and breaking the tests.

See: #5734
ncw added a commit that referenced this issue Oct 20, 2021
In some backends (eg putio) this deadline was consistently missed at
10s so this patch increases it to 30s.

See: #5734
@ncw
Copy link
Member Author

ncw commented Oct 20, 2021

@buengese I had a quick go at fixing 1fichier in 9742648 - my local tests indicated more list retries will fix the problem - we'll see tomorrow!

ncw added a commit that referenced this issue Oct 20, 2021
The test TestIntegration/FsMkdir/FsPutFiles/FromRoot/ListR fails in
the integration test because there is a broken bucket in the test
account which support haven't been able to remove.
@ncw
Copy link
Member Author

ncw commented Oct 20, 2021

32 failing integration tests this morning so gettng there! I've merged a lot of little fixes so hopefully that will be much less tomorrow 🤞

I think I've reviewed everyone's pull requests now (except for the ones explicitly marked for 1.58) - let me know if I've missed anything.

@ivandeex

This comment has been minimized.

@ivandeex
Copy link
Member

@ncw Oops I missed your inline comment in #5462 , sorry. Will keep the latest tag.

@ncw
Copy link
Member Author

ncw commented Oct 21, 2021

@albertony - I noticed that TestTouchDir that was merged as part of 41876dd broke 6 integration tests this morning :-(

I think you need to not run this test on backends which don't support modtime (check the features flag and t.Skip - there are lots of examples of this in the tests) - these tests will always pass because the acceptable mod time range is infinite so there is no point running them.

And also t.Skip if the operations.TouchDir returns either of these errors - these are backends which only support modtime when you upload a file and can't change it afterwards which is fine for sync purposes but terrible for touch!

rclone/fs/fs.go

Lines 32 to 33 in 1a66736

ErrorCantSetModTime = errors.New("can't set modified time")
ErrorCantSetModTimeWithoutDelete = errors.New("can't set modified time without deleting existing object")

dropbox: 2021/10/21 05:47:35 ERROR : sub dir/potato3: Failed to touch can't set modified time without deleting existing object
koofr: 2021/10/21 06:07:45 ERROR : sub dir/potato3: Failed to touch can't set modified time without deleting existing object
pcloud: 2021/10/21 06:34:46 ERROR : sub dir/potato3: Failed to touch can't set modified time
pcloud: 2021/10/21 05:35:40 ERROR : potato2: Failed to touch can't set modified time
webdav: 2021/10/21 05:36:50 ERROR : sub dir/potato3: Failed to touch can't set modified time

@elek
Copy link
Contributor

elek commented Oct 22, 2021

Hi, Is there any chance to merge #5646 (and maybe #5613) before the release?

The discussion of #5646 is already concluded, but there is a small code parts (enable Storj S3 compatible configuration as a new S3 provider).

#5613 Is just a simple rename (s/tardigrade/storj) but I can rebase it only after #5646

Thanks a lot...

@ncw
Copy link
Member Author

ncw commented Oct 22, 2021

@elek - let's try to get #5646 into 1.57 - I'd like to leave #5613 for 1.58 as there is more potential for disruption there.

@ncw
Copy link
Member Author

ncw commented Oct 22, 2021

@buengese I figured out what was wrong with Zoho - my free trial has expired! The errors were upload error: HTTP error 402 - I had to look that up it means "payment required". I've asked them if they can extend the free trial for the rclone project. Or if that fails do you have an account?

ncw added a commit that referenced this issue Oct 22, 2021
The API has changed in the directory move call JSON response from
returning a TaskID as a string to returning it as an integer. In other
places it is still returned as a string though.

This patch allows the TaskID to be an integer or a string in the JSON
response and keeps it internally as a string like before.
@ncw
Copy link
Member Author

ncw commented Oct 22, 2021

The same thing is wrong with premiumizeme also - I've emailled to ask for my account to be re-instated.

Phew - it is a lot of work keeping all these accounts going for the integration tests!

ncw added a commit that referenced this issue Oct 22, 2021
This makes sure the namenode is accepting TCP connections before
starting the integration tests in an attempt to make them more
reliable.
ncw added a commit that referenced this issue Oct 23, 2021
…5734

This needs fixing properly so rclone knows when the server has started
properly.
@ivandeex
Copy link
Member

ivandeex commented Oct 25, 2021

Master commit 00ceeef test failed due to serve/docker race in the quick test. The quick test race is rare compared with race in the race test, usually fixed by restarting the test. Will be addressed by #5738.

ncw added a commit that referenced this issue Oct 31, 2021
Apparently moving a directory using the id "0" as the root no longer
works, so this reads the real root ID when it is listed and uses that.

This fixes the DirMove problem.

See: https://forum.rclone.org/t/premiumize-cant-move-files/27169
See: #5734
@ncw
Copy link
Member Author

ncw commented Nov 1, 2021

After a lot of bashing on bugs, the integration test scores are as follows...

backend remote test failure comments
compress TestCompressDrive: backend/compress FsChangeNotify @buengese - not that important
fichier TestFichier: backend/fichier Lots API rate limiter is killing the tests
filefabric TestFileFabric: fs/sync Didn't finish Not finishing in 2hr time limit
mega TestMega: fs/operations Login Fail Test account over API quota
onedrive TestOneDriveBusiness: fs/sync TestSyncConcurrentTruncate @ivandeex investigating
onedrive TestOneDriveCn: backend/onedrive All test account broken
uptobox TestUptobox: fs/sync TestSyncConcurrentTruncate @ivandeex ?
zoho TestZoho: backend/zoho All test account broken

I deem these good enough to make a release :-)

I'm going to merge any doc fix PRs that looks safe then make the release!

@ncw
Copy link
Member Author

ncw commented Nov 1, 2021

Release released!

Minor glitch on webside: https://rclone.org/downloads/ which I'll see if I can fix in a moment!

image

Release announcement here: https://forum.rclone.org/t/rclone-1-57-0-release/27244

@ncw
Copy link
Member Author

ncw commented Nov 1, 2021

Fixed above website glitch in 19dfaf7

I'm declaring this done!

@ncw ncw closed this as completed Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants