search

the-scouts / compass-interface-core

Core bridge/crawler component for Compass-Interface
5 stars 1 forks source link

Parsing error when role end date set #52

Closed rglss closed 3 years ago

rglss commented 3 years ago

When a role is full, but has an end date defined (i.e. CC) this throws a parsing error:

unexpected value; permitted: 'Cancelled', 'Closed', 'Full', 'Pre provisional', 'Provisional' (type=value_error.const; given=Full Ending DD MMM YYYY; permitted=('Cancelled', 'Closed', 'Full', 'Pre provisional', 'Provisional'))

AA-Turner commented 3 years ago

Version? Should have been fixed in 0.9.5

AA-Turner commented 3 years ago

Here: a02dff7, PR #49

rglss commented 3 years ago

0.9.6 - Something's been lost somewhere it seems...

AA-Turner commented 3 years ago

Is the text exactly Full Ending with no left paren? If so it won't match. But that is odd, as you'd Compass would be able to serialise ending dates consistently...

rglss commented 3 years ago

Ahh found it - this error is being thrown on get_roles_tab rather than get_training_tab where the above patch is applied.

https://github.com/the-scouts/compass-interface-core/blob/098beca7a745ff6e09506e8cd1335a20f8e76cb1/compass/core/_scrapers/member.py#L289

AA-Turner commented 3 years ago

PR to fix - although bit unusual to have an end date in 2024 - most I see are for less than a year!

AA-Turner commented 3 years ago

Released v0.9.7 to fix.

rglss commented 3 years ago

See PR comment - I don't think this patch will resolve the issue

AA-Turner commented 3 years ago

Released v0.9.8 to fix the fix that didn't fix things.

rglss commented 3 years ago

Soz - I had a PR ready to roll but your first try beat me to it. Just too damn efficient Adam 😆

AA-Turner commented 3 years ago

Should have said - I'd've held off!

Sorry for the ineptitude - but an interesting design decision to use parens in one place (training tab) and not in another (roles). I hope that the same data backs both, otherwise we may find some interesting data-sync bugs!

rglss commented 3 years ago

Yer, it really is. The good news is out of the whole county thats the only issue i've encountered!