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

Add xandikos to CI #601

Merged
merged 11 commits into from Mar 22, 2017

Conversation

Projects
None yet
3 participants
@untitaker
Member

untitaker commented Mar 14, 2017

Fix #597

Bugs:

Other chores:

  • Add to list of supported servers

@untitaker untitaker force-pushed the xandikos-ci branch from 107d02d to 6382b01 Mar 14, 2017

@untitaker

This comment has been minimized.

Member

untitaker commented Mar 14, 2017

Xandikos 404s when trying to list all collections, also it does not properly differentiate between addressbook-home-set and calendar-home-set, which means that MKCOL on those URLs will not work properly since the server won't know which collection to create.

This is RFC-conformant but unusuable behavior. Same problem as Radicale.

@WhyNotHugo

This comment has been minimized.

Member

WhyNotHugo commented Mar 14, 2017

Xandikos 404s when trying to list all collections

Oh, I thought I was doing something wrong when I approached this, so it wasn't my bad. :(

also it does not properly differentiate between addressbook-home-set and calendar-home-set, which means that MKCOL on those URLs will not work properly since the server won't know which collection to create.

I'd like to understand this a bit more (and also, forgive my slight ignorance here), but, what don't we includ DAV:resourcetype in MKCOL? Shouldn't that suffice to identify the collection type?

@untitaker

This comment has been minimized.

Member

untitaker commented Mar 14, 2017

IIRC MKCOL with request body was rejected by other servers.

@jelmer

This comment has been minimized.

Contributor

jelmer commented Mar 15, 2017

Listing calendars with vdirsyncer works fine against xandikos for me; what request 404s for you?

MKCOL with a body indeed allows specifying what resource types the new collection should have. That said, Xandikos doesn't yet support setting the resource types of newly created collections. jelmer/xandikos#16. It's correct that some servers reject a MKCOL with body; servers should reject a body they don't understand per RFC4918; RFC5689 defines an XML body for MKCOL.

I've filed jelmer/xandikos#21 about automatically setting the type of collections based on under what path they were created, if there was no body specified for MKCOL.

@untitaker

This comment has been minimized.

Member

untitaker commented Mar 15, 2017

I think there's a misunderstanding in how I should set up xandikos for testing, could you review this PR?

@untitaker

This comment has been minimized.

Member

untitaker commented Mar 16, 2017

@jelmer Actually we send a MKCOL with all the properties that are necessary for xandikos to infer the collection type: https://github.com/pimutils/vdirsyncer/blob/master/vdirsyncer/storage/dav.py#L266

@untitaker

This comment has been minimized.

Member

untitaker commented Mar 16, 2017

Turns out I wasn't setting up Xandikos correctly. We're now able to create collections properly, but the tests still don't pass. I will file issues on Xandikos for that.

@jelmer

This comment has been minimized.

Contributor

jelmer commented Mar 18, 2017

With current vdirsyncer and current xandikos, all tests now pass for me.

.travis.yml Outdated
"env": "BUILD=test DAV_SERVER=radicale REQUIREMENTS=minimal BUILD_PRS=true ",
"python": "3.3"
},
{
"env": "BUILD=test DAV_SERVER=xandikos REQUIREMENTS=minimal BUILD_PRS=true ",
"python": "3.3"

This comment has been minimized.

@WhyNotHugo

WhyNotHugo Mar 19, 2017

Member

Xandikos only support python 3.5 or above:

https://github.com/jelmer/xandikos#dependencies

This comment has been minimized.

@untitaker

untitaker Mar 19, 2017

Member

@jelmer Would you consider supporting Python 3.3+ (i.e. I submit PRs to xandikos if Python 3.3 breaks)

@untitaker untitaker force-pushed the xandikos-ci branch from 15b96ad to 31b2297 Mar 19, 2017

@jelmer

This comment has been minimized.

Contributor

jelmer commented Mar 19, 2017

@jelmer

This comment has been minimized.

Contributor

jelmer commented Mar 20, 2017

Dulwich & Xandikos now support 3.3. That was way easier than I was expecting, no changes necessary.

@untitaker untitaker force-pushed the xandikos-ci branch from 08ec90b to 05e0fa1 Mar 21, 2017

@untitaker

This comment has been minimized.

Member

untitaker commented Mar 21, 2017

@jelmer the tests are failing for anything below 3.6. It appears there is a bug in the urllib module that makes xandikos output double slashes for calendar-home-set

@untitaker

This comment has been minimized.

Member

untitaker commented Mar 21, 2017

One of your recent commits breaks the CI again :(

@jelmer

This comment has been minimized.

Contributor

jelmer commented Mar 22, 2017

Argh, fixed now.

Another reason to extend the unit test coverage further...

@WhyNotHugo

This comment has been minimized.

Member

WhyNotHugo commented Mar 22, 2017

I wonder if, as a way to get around the temporarily lack of coverage, you could run vdirsyncer's test suite for Xandiko's CI. :P

@untitaker untitaker merged commit 28f2041 into master Mar 22, 2017

1 of 2 checks passed

codecov/project 83.01% (-0.51%) compared to bb4a9a1
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@untitaker untitaker deleted the xandikos-ci branch Mar 22, 2017

@untitaker untitaker removed the in progress label Mar 22, 2017

@WhyNotHugo

This comment has been minimized.

Member

WhyNotHugo commented Mar 22, 2017

Thanks all! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment