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

Feature/image attachment support #9

Merged
merged 22 commits into from
Apr 7, 2018

Conversation

scolby33
Copy link
Owner

Supersedes #5 since it has been dormant for a while.

  • Refactored image attachment support just a tad
  • Added tests

@@ -15,6 +15,8 @@
TEST_URL_TITLE = 'Reply to @someuser'
TEST_SUBSCRIPTION_CODE = 'Forum-f504h08fhlasdfj'
TEST_SUBSCRIBED_USER_KEY = 'sPfjsD2fGzEd9TR52DU31Hv4A61Vvk'
# the Pushover logo
TEST_IMAGE_BYTES = b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\xa6\x00\x00\x00$\x08\x06\x00\x00\x00\x03\xf0A\xa4\x00\x00\x00\x06bKGD\x00\x00\x00\x00\x00\x00\xf9C\xbb\x7f\x00\x00\x00\tpHYs\x00\x00\x0b\x13\x00\x00\x0b\x13\x01\x00\x9a\x9c\x18\x00\x00\x00\x07tIME\x07\xdf\x0b\x03\x022\x05\xd4G\xee5\x00\x00\x0c\xddIDATx\xda\xed\\i\x94T\xc5\x15\xee\x11\x10\x15wQ\x82(\x1a\\\x00G\x05\x05\x82h\x88\xeb\t\xc9qA\r.\xa0&\xc4@\\B\\\x8e!\x9aDc+\xb2\x88\x1e\x05\x17\xa2\xacQ\xc0\xe8\xa8\x1c\xa3\x1eD#\xd2\x12\x17\x90I\xa2\x07;\x8a\x13ddd\x1c;=m\xcft\xf7\xeb\xf7^\xd5\xfd&?R\xef\x9c\xa2\xac\xaa\xf7\xde,\x98\xc9\xe9\xfa\x07\xefvU\xbd[\xb7\xee\xfd\xeew\xef\x9b\xaaD\'\x0c\xd7u\x07\xf5\xec\xd9sHUU\xd5\xc1\x89D\xe2\xe0D"\xb1["\x91\xc8\xb6\xb5\xb55\xe6\xf3\xf9\xb7\xfa\xf6\xed[LTFet\xf5\xc8f\xb3{\x13\xd1d\x00\xcf\x00hl\xb3\x0c\x00\x1e\x80\xd5\x8c\xb1Q\x15\xcdUF\x97\x0c\xc6\xd8H\x00\xcb\x00\x14%\xc3s\x01l\x04\xf0"\x80\'\x00\xac\x02\xf0\xbe\xc1Hg\xfd?\xea%\x9dN\xf7r]\xf7\x88\x8a\x85\xec\xe2\xe1y\xde`\x00\xcfI\x06V\x06\xb0\x84s~fCC\xc3\x1e\x86\xf0~$\x80?\xab\xc6ID\xb7wW=455\xedEDW\x10\xd1\xed\x00\x96\x02X\x07\xe03\x00\x04\xe0\xa9\x8a\xa5\xec\xa2Q[[\xdb\x13\xc0,\x00L\x18$\x01x\xa8P(\x1c\xa4\x93\xaf\xab\xab\xdb\xbdP(\xf4\r\xfe\x9dJ\xa5z\x00X\xa7xM\xb7\\.\x0f\xec\x8e\xfa \xa2_\x9a \x0bclD\xc5bv\xc1p]w\x10\x80\r\x92Amg\x8c\x9d\x92H$\x12\x8e\xe3\xf4\x17\x9e\xe3\x0e\xe19R\x00\xb6\x03 "\xfa\x99<\x0f\xe7\xfc"\x8d\xd7\xbc\xb1\xbb\xe9\xa3\xa6\xa6f7\x00\xff2@\x94T\xc5bv\xc1\xf0}\xffd\x00\xcd\x92\xe27;\x8e3 x\x0e\xe0)\xc3\x01}Y__\xdf[\x99k\xb8Fnyw\xd3\x89\xee\x82\x05\x83s~A\xc5j\xba>\xc1\x19\r\xe0+\xd9S\x16\x8b\xc5\x83\x83\xe7\xe5r\xf9\xb0 \xb4k\x0c.\xa9\x99\xefT\x8d\xdc\x0b\xddM/\x00\xd6\x1b\xdeyK2\x99\xac\xaaXN\xd7&9G+F\xe9\xaa4\x0f\x80\xb9\x86\x03r\x8b\xc5\xe2!\x1a\\v\xb5Fvi7\xbb\xac#L\xde\x92\x88\xae\xadXN\x17\x0e\x91\xa8\xfcM1\xa0\x99\xb2L6\x9b\xdd[6\\Ev\x89\xc1\xd3\xd4h\x0e\xf3\xd6n\xe6-M\xd0\xe5\xdf\x8d\x8d\x8d{V\xac\xa7k3\xce\x9f+J\xcf\xe5\xf3\xf9\xfd\x15\x99\x1bL\x9e\xc3\xf7\xfd\xe3u\xf4\n\x80\x82*\xeby\xde\xd0o\xf2]\x93\xc9dU\xb9\\>\x8c16\x96s~\x9eM6\x04\xba\xdc]\xb1\x9c\xae\xf7\n\x1f+J\x9f\x13#+}\xcd`\xec\xd34\xb2\x1f|\x03\x10e(\x80\x05\x00^\x01\xb0\x05\x80+\xed\xe7\xa5\x10\xbd\xdck\x82.\xa5R\xa9\x9f\x1cM<\xcf\x1b\xec\xba\xee\x91\x1d\xc5\x9c\xc9d\xb2\xaaX,\x1e\xe2\xfb\xfe\t\x8c\xb1\x11:\x88\xd4Ah2\x06\xc0j\x00\xf5\x006\xc5\xa1\xbaR\xa9T\x0f\xc7q\xfa\xfb\xbe?\xdc\xf7\xfd\x93;ko\xf5\xf5\xf5\xbd}\xdf\xaf\xf6<o\x88|pC4\xbc\xdc\xe8\x18Y\xe9\x0f\xd4\x85\xd2\xe9t/\x00\x9fj\xc2\xf8%\x89D"\x91\xcf\xe7\xf7\x97+H\xca\xa1/\xd2\x18\xc83\x06\xd9lSS\xd3^!\xc6\x95\xb2\x94L\xef1\xfd.\x93\xc9\xf4\xb1@\x97\xc5\r\r\r{\x10\xd1\x8d\x006\x01 \x99\x9d \xa2\x9b\xe2\x1a#\xe7\xfc|\x00+\x00d5\xeb\xbd\xe3\xfb\xfe\x89\xa6\xdfG\xd5\xa7\xef\xfb\'\x02(+\xcf\xdf\r3F"\xba\x0c\xc0\xb3\x00Z\x0c{\xab\x8e\xb9\xa7\x85\xc2\xf6\x8e\x01\xb0\x08\x80#={9\xf0l\x13\x95\x1f5\xaa\xb7\x1e\xc0_\r\x0b\xa4u\x1e\x82\x88n\xd5\xc8n\x94\x9e\xff\xda\x02\x0b\x8eS\xc2\xe9@\x00\xdc\xb0\xfe\xcc\x10\x88r\xa9\xad\x96\xcf9\x9f`\xf9\xed4\xc3\x9a \xa2\xab\x01\xd4\xd9\xe6&\xa2i\x11\xa9\xa8\xef\x9b\xca\xb8\xca\xba\x19\xb9\x80\xa1\xec5\x92>\x01<\xa6\x9b\xd7\xb2\xb7\t\x00>\x89\xb0\xb7\xad555\xbbE\xd9\x13\x00\x88\x0br\x97\x1c\xbd\xe4\x11Lp\x95\xf2\xc3\xa7\x15\xd7?\xca\xa2\xfc\xa9\x1ar\xfe\xdb\xeaM\x01P\nn\x95\xa8(m7lz\x8d\xc6\xe3\xddg\x90\xf5\x1d\xc79\xd4\xa4\xd4\xc6\xc6\xc6=\x01|fS\xa8\xe7y\xc7X\x08\xf5:\xc3\xba\x0e\x00/\xc2am\xb4\x19dCC\xc3\x1e\x00\x1e1\xfc\xf6s\x01?^W\xf4\xfd5O\x1cG\x9f\x00Vkd\xdeW\xe7lnn\xde\x07\xc0r\xc3\x9cO\x88\xcb\xb4Sb+s\xdd!{\xfa\x08@\xadMw\x89D\xe2\xbf\x95\x1c\x00y\xe9\x87\xf7*/\xf3\'SV\xaa\xd6\xca\x85\xb2\xff\xae1\xe0\xc9\xd2M\xba\xdc\xe2\xc1\xc6i\x98\x80\xbca\xfd\xe5!!<\x19b8%\xf5\x96K\x9e\xe2\xc2\xb6\x0e\x0e\x00[BB\xaf\x89\x1b}2\x80\'\x82-\x91\x9bf\x96i\xbced}\x12\xd1\xa5\x00\xb6\xca\xf0F\x9d\xb3X,\x1e\xa224B\x8e+\xe7\xb8SD\xc9f\xb3{G\xd9S\xe05\x01\xbc\x07`\x8b\xe6\x19\xc98\xf3X\x00\x0b\x01|\x08`\xae\x14F\x0f\x8f\x9a\x95\x8a[\xf2\xacF\xeeN\xc5`6\x9a`\x81F\xe96&\xe0dK6=Px\xb6\x16\x8br\xde\xb3\x18\xf5\x9b\x96\xdf}ND\x97d2\x99>Dt\x99E\xeeu\x83Q\xeeg\xe9\xc0Z\xae\x81Q\xad\xd2\xf3\x15\x9a\xbdF\xd6\xa7t\xf1\xce\x91\x9c\xc6\x15\xc1\xff\x17\n\x85\x83\xd4D\xd8\x14\x1de\x1a\r\xc0\xb6({\x12\xb2\x7f\t\x92\x1c\x9d\x01Gj\x88\xb1\x84\xd1\x9d\xb2\xd2\xfa\xfa\xfa\xde\x00\x9e\x0f\xe3,\x19c\xa7E\x85\x05!L\xc0\x9b!\xfb~F\xc8=lQ\xd0\x92\xb8\x84:\x80\x1d\xe5r\xf909A\xb2\xc8\xce\xd0e\x9f\xa6d\x0c\xc0\xda\xda\xda\xda\x9eJ\xf2\xf5\xa4"3?\xac\xb2f\x83Y\x92~\x16\x8b\xf9X\xd0\x94#\xa2\xdd\xdb\x86\xbd=\xac\xf1\xf8\x05\xe9\xf9\xec(g\x0c`\xa6|\xf18\xe7\xe3T\xa3\x95=\xafv\x84\x84\xd1e\x92\xb7\x1d\xacz\x00\x00_\xea8B\xb9}.\x0c\x16\xd8\xc2)\xe7\xfc\xc20o\x00`\x13\x11]i9\xb8[\x0c\x87\xb6\xd2\xb0G\xa6R+\xbe\xefW[:\x8eFi\xe6^`\x98\xbb\xa9T*}K\x91}Tgl\xcd\xcd\xcd\xfb\xf8\xbe?\\\x97`\xda\xf4\x19\x8c\\.\xb7o\x10I\x00\xbc"\xad\xf7\xb8a\xae\xcdj\x0f\x84\\\x01\x04P\x92;\xc6\xe2\x9cqmmmO"\xba\x05\xc0\\\xce\xf9\xf8\xa8\xa4\xfb\x8d\x960:\xcc\xf7\xfdj\x00\x8f\xcb\x89@\xd0\x9b({S)):\xd2\x92]\xcf\x88\x1aNu\x19\xa0r\x99\xb6\x01`\xbe\xef\x9f\x04\xe0n\x8bq\xffP\xfd\xbd\xe38\x03\x00\xf8\x86u\x17Da\x1f\x84\xec\x87\x9a\x0b\x13\xb9\x11DD\x0b\x1d5C\x11\xf1\xed\x0c\xcb\xb9\xde*\x19\xfaD\xb1\xb7\x8b\r\xf3P\xd0U&y\xc4\xef\xca\xe7HD\xb7E<\xe3\x8e\x17$\x84b\xb6\x1a\x16\xe0\xaa\'\x15\x9d\xec+v"H\xbfnh\x0f\x9a>\xbfP\xbdEH}\xfa\x06\xcb\x1a\x8f\xca\xa1\xc5\x94\xb8\xb5\xb5\xb5\xb5\xb9\xae;H\xf3\xfb9\xa6=:\x8e\xd3?\xac0!\xed\xf1zM\xe8\xfb\xc20\xf7\x0b\x06\x0e\x95\x0c\xf2_\x89/\x05\x1a\xa2\xeaS\x81\x12_\x04^\xba\xae\xaenw\x81yM{{B)V\x0c\x06\xd0$=\x7f^v\x12\x00\x1e\x88\xbb\xa7X\x83s\xfe#\x1b\xf8\x17)\xffZ\x00\x8f\x10\xd1\xe5\xf9|~?\xdb|"|\xb4Fyy\xf1\x82+\x0c\xb2y\x13\x06\xe1\x9c\x9f)\xb2\xbd\x7f\x04\xa1G\x97]\x06\x17I\xf5\xba\xc2\x18r\x06\xf9?j\xd6\x1b\x17\xb5\x86n\xc2\xba\x00\x98\xee\x82\x08\xcf6]\xa5~|\xdf\xafN&\x93U\x82\xcei\x89\xaaOi\xcek\xd4n0\x00\x0f\x99\x1c\x90\xe7yGK\xef{:\x80/\xa5\xe7\xaf\xc8\xa1Y\x86\x08Q\xf4\xd7\xde\x12\xa5\t\x04\xd7\x99\xc2h\x08,\xb8\xd9\x02\x0b\x86+\xe1\xf4PK8\xbd_7\xbf\xa08v\x00(\xc9^\xdb\x82\x917k\xf6\xf8\x0b\x1bt\xd1\xe8\xe8\r\xc3\xdcw*\x10f\x90\xe5}\x16[\xce`\x99\x12\xee\xcf\x89\x08\xb3\x86[\x9cC\x93X\xb7\xa5\xa5\xa5\xe5\x00\x11z=\x9b1\x89~\x81\xc7\xe4\x10\r`^*\x95\xea\x11\xe3\x8c\x87uF-\xf5;\x1d\xadfh:\x97\xb6\x19^~\x9d\xe6@f\x99n\xb0\xee\xa3/\x01;^S3\xd1\xd6\xd6\xd6\x03-^\x7f\x95f\x8eOL\xd9\xb2FG#M\xdc\xa8Z\x9d\x01\xb0\xc4\x84\xdf\\\xd7=J\xa7\xb3B\xa1\xd0W)\xd3m\x0b\xb2\xd9\x10\x98\xb5\xceb\xe8\x0fHrw\x99*A2\xc4\x00\xf0\xa6Rn\xfd\x88s~z\xcc3^\xdbY\xde\xf2i\xc3\x02\xb9L&\xd3\xa7\x1d\xb0`BT\xd0/:\x92\x9a\r\xeb\xd7\x18\xbc\xf1\xef\xc5\xf3\xe7b\xd0>3\x95=\x8e\xb7\xec\xf1\xbc\x18\xb5\xfby\x1aO\xee\x1adW[\xce`\x8e\xe2\x10nkO\x12%\xb1\x07\xc7I\xdfn\xed\xc8d2}Z[[\x0f\x94\x8d\xdf\xa2+\x02\xb0\x9es>A\xf5\x92Q\xa0_X\x07W\xa4!\x88i\x13\xa1>\xa7\xaba\x01\x11]g\xa1_\xc6h\x8cx~p\xa3U\x8e\x8f\x88&Y<\xff\x95Q\x1a=t\x1d\xea\xa2\xe4\xcau\x00_\xe68u8Q9\xb0\x8bu\xfa\x12PF\xae\xf6dd\\m\xa9\x18ia\x96(|\xbc+\xbd\xfbUa\xc5\x0b\x00\xaf\x02\x98MD\x93\xe4/\x18\xdaq\xc6\x9d\xd3\xe1\x0f\xe0~K]z@;`\xc1\xa8\xa8\xb0@\x84\xa8\x8f\r\xebo\xd0\xd0B*w\xfa\x85\x89k\xd3`\x9e\x13\x82\xae\x1e\xdbeP\xb3\xeb\x90da\xa9F\xd6Xo7uE\xa9\x8d\xc9DtsD\xb6b\x9aa\xbeY:/-\x1b\xab\nG\xe24@\xab\xbd\xbca\xfa\x8b=D\xa6\xd7\xae\xbatT\x00o\xcb\xae\x89h\xb2\xe5\x05\'*\xf3>\x16V\x9d\xb0x\xc1|*\x95\xea\xe1\xba\xee\x11j\x93\x84\n]T\xe3\x11\tD1J\x89\xd4q\x9c\x016\x8f\x14%$\x02H\xcb\xe4\xb6\xa5\xb1\xe2+\x1d[!G\r\x00\xb9\x80\xf2\x12\x18\x96\xe2B\x0c\xb5U\x0f\xc0\xec8\xfak\xd7 \xa2\x9b,\x1e\xe6\xa4\xb8\xf3\t\xfa\xa5\x14%\x1b-\x95J\xfd\x00\xec0\xc8\x16\xe4\x1b\xec8N\x7f\xe5y\x19\xc0JYF\xccg\x82$/\x89\xef\xe5\x9d\x10l5[\xa3\xa3\xa9\x06\xd9O4\x97\xb2\xc62\xf7\\MC\xf3\x10\x99\xaeR+M---\x07\xa8\xbd\x94\xa6Re@\x9a\x07l\x00\x00\x92\x0b\n\x9c\xf3s-{\xbb7\xecl\x05\xf7\xb9*\xae\xfeb\x0f\x91U}\x1a7\xd3\x0bIz\xc6[<\xe0\xb5\xcaKn\xb2\xbc\xe0\x1b\xca\x81\xcf\xd3Q(\x8a\xcc\xc2\x08\xa0\xfeE\x00\x8b\xe2@\x17\x00k\x0c\xf2\xcf\xaa]\xe2!\xbd\x9a\xbf\xd3\x18\xe5g\x8a\xccobt\x11\x8d\x97=\x19\x11M\x97/&\x11MW\xe6\xfa\x95E/w\x86\x9c\xeb8\x00\r")j\xecL\xe8\x177s>\xbf\x9da\xfcn\x1b\x15\xc1\x18\x1bCDSM4\x83Rw\xbf\x801v\n\x11\xfd\xd6\xc4\xedI\x87r[\xc8|\xcdD4E$O&B}\xa5!t\x99:\xb2\x9b8\xe7\xe73\xc6N\x03\x904E\n\x99G\xf5}\xbf\xdaq\x9cC\x89h\x9a\xda)o\xe8\xe2\xbf\xcf2\xdfb\xa1\xcf\xc9\xea%\xd7yg"\xba\xc32\xd7[\xe9t\xba\x97\xa6\xc3\xfe\\\x00\xaf\x068\x94\x88&Y:\xfcWv\x16E\xf4NggU6\x8eL\xb3\xce\x83&\x9a(\xe4w\xf5D4\x9d\x88\xa6\x00\xb8\xc7\xd6\t.J\xa9\x0b\x83,\x93\x88\xae\xb70\x00#t\x04u\xcc\xbd\xe5\xdb\xd9\xc7Y\xa3\xa3e\xd4N\xa3\x88s\xcd\x0e\xab\x00\x99\xf4*>\xf3X\n\xe0e\xa5\x0c\xb9\x9516\xd2\xd4\xe1o\xd2_{\x08\xf5\xd1\x96\x90s]\x07\x8c}VD\xe5\xcd\x13\xde\xa81\x82\xac#\xfe4\rb\x1e\xd0\x1a\xcf\xf3\x06+\xde\xcfD\xa8\xaf\xb7\xf4\x0f\x14#\xae7\xcbu\xdd\xa3L\r\r\x96\xe6\xd9\xbbL\x8e\xc0\xf4a\x9ca\xae"\x11M2\x9d\x8d\xad\xd9"d\x7f\xcbr\xb9\xdc\xbe!\x1d\xfe\xeb;\xcb[\xd6\x18\x16\xc8v$\xab\xf2}\x7f\xb8\xad\x1bF(oJX\xc6)\xc9\x7f\x10|\x9ea\x0bkj\xb6\xaa\xfb\xa3\x04\x9c\xf3\x0b,\xd0\xe5"\x8b\xae\x16D0\x88\x1fK\xf23"\xees3\xe7\xfc\xec\x10}\x1eo*m\xaa\xd8\xd9\xf4\xc9\x88\xf2.\xf7\xc40\xca\xb5r\x1b_{\xf5\x17y\x08\xca\xa4]\x1fzE\xcc\xf4/\x05\x90\xd1t\xc7<\xa2\x82c\x91I\xbf\xad\xb9\xa5\x1b\x88\xe8\xa7\x9a\xda\xec5\x00\xeau\xc6\x01\xe0u"\x9aj\xe1\n\xd7\xc5m\xa9\x93\x9ai\x17\xa9\x06"\xde\xe9\x0f:\xc0OD?\x01\x90\xd65p\x00XCDW\x9a**\xba^S\x00\xff4t\x1c-e\x8c\x8d\x8dy>S-\xe5\xcd\x8f\x00\xcc\x0f8\xdf\xce\xd0\xdf\xff\xd4H\xa7\xd3\xbd\x18c#9\xe7gz\x9e74\xec\x10<\xcf;\x96s~6cl\xb4\xfaG\x17\x0c\xf2G3\xc6\xbe\xc7\x18\x1b\xeby\xde\xb1r\x17xW\x8d\\.\xb7/cl4\xe7\xfc,\xdf\xf7\xab\xa3\xacY*\x95\xfa1\xc6\xc6p\xce\xcf\xf2<ohG\xa2\x91\xeb\xba\x838\xe7g0\xc6N-\x97\xcb\x03\xa3\x1a\xb6\xa5\xe2w8c\xecT\xf1>\xc3L_c\xee\xaa\xf1\x1f9\xec5\xd4\xfe\r\x9d\x05\x00\x00\x00\x00IEND\xaeB`\x82'
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should find a better/smaller test image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like a 2-by-2 square might be better

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like the smallest valid ELF, someone has done this: http://garethrees.org/2007/11/14/pngcrush/

And of course there's mathiasbynens/small which is the first google result for "smallest png"

@@ -46,6 +59,10 @@ def messages_callback(request):
resp_body['token'] = 'invalid'
resp_body['status'] = 0
resp_body['errors'] = ['application token is invalid']
elif qs.get('attachment', None) and qs.get('attachment', None) != TEST_IMAGE_BYTES:
resp_body['attachment'] = 'wrong image' # todo better error message like real Pushover does
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are not the real error messages. I need to investigate the Pushover API to see what it returns in various cases.

try:
from urllib.parse import urljoin
from pathlib import Path
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cthoyt What was your thought on try/except imports? Where should they be located?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always put them at the bottom of the normal style imports. Does six not have something for Path? Or is this a situation where pathlib2 is an improvement?

I think we're in python 3 only here, so why are you even using six?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JK not python3 only.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Six doesn't have pathlib and also doesn't have changes between file and the new _io which is the real problem

@scolby33 scolby33 mentioned this pull request Feb 22, 2018
@codecov-io
Copy link

codecov-io commented Feb 22, 2018

Codecov Report

Merging #9 into develop will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           develop     #9   +/-   ##
======================================
  Coverage      100%   100%           
======================================
  Files            3      3           
  Lines           97    147   +50     
  Branches         7     11    +4     
======================================
+ Hits            97    147   +50
Impacted Files Coverage Δ
src/pushover_complete/pushover_api.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9a27b97...0dd9424. Read the comment docs.

@scolby33
Copy link
Owner Author

Also add test for #7 before merging

Copy link
Contributor

@cthoyt cthoyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check all comments for specific changes

try:
from urllib.parse import urljoin
from pathlib import Path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I always put them at the bottom of the normal style imports. Does six not have something for Path? Or is this a situation where pathlib2 is an improvement?

I think we're in python 3 only here, so why are you even using six?

@@ -15,6 +15,8 @@
TEST_URL_TITLE = 'Reply to @someuser'
TEST_SUBSCRIPTION_CODE = 'Forum-f504h08fhlasdfj'
TEST_SUBSCRIBED_USER_KEY = 'sPfjsD2fGzEd9TR52DU31Hv4A61Vvk'
# the Pushover logo
TEST_IMAGE_BYTES = b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\xa6\x00\x00\x00$\x08\x06\x00\x00\x00\x03\xf0A\xa4\x00\x00\x00\x06bKGD\x00\x00\x00\x00\x00\x00\xf9C\xbb\x7f\x00\x00\x00\tpHYs\x00\x00\x0b\x13\x00\x00\x0b\x13\x01\x00\x9a\x9c\x18\x00\x00\x00\x07tIME\x07\xdf\x0b\x03\x022\x05\xd4G\xee5\x00\x00\x0c\xddIDATx\xda\xed\\i\x94T\xc5\x15\xee\x11\x10\x15wQ\x82(\x1a\\\x00G\x05\x05\x82h\x88\xeb\t\xc9qA\r.\xa0&\xc4@\\B\\\x8e!\x9aDc+\xb2\x88\x1e\x05\x17\xa2\xacQ\xc0\xe8\xa8\x1c\xa3\x1eD#\xd2\x12\x17\x90I\xa2\x07;\x8a\x13ddd\x1c;=m\xcft\xf7\xeb\xf7^\xd5\xfd&?R\xef\x9c\xa2\xac\xaa\xf7\xde,\x98\xc9\xe9\xfa\x07\xefvU\xbd[\xb7\xee\xfd\xeew\xef\x9b\xaaD\'\x0c\xd7u\x07\xf5\xec\xd9sHUU\xd5\xc1\x89D\xe2\xe0D"\xb1["\x91\xc8\xb6\xb5\xb55\xe6\xf3\xf9\xb7\xfa\xf6\xed[LTFet\xf5\xc8f\xb3{\x13\xd1d\x00\xcf\x00hl\xb3\x0c\x00\x1e\x80\xd5\x8c\xb1Q\x15\xcdUF\x97\x0c\xc6\xd8H\x00\xcb\x00\x14%\xc3s\x01l\x04\xf0"\x80\'\x00\xac\x02\xf0\xbe\xc1Hg\xfd?\xea%\x9dN\xf7r]\xf7\x88\x8a\x85\xec\xe2\xe1y\xde`\x00\xcfI\x06V\x06\xb0\x84s~fCC\xc3\x1e\x86\xf0~$\x80?\xab\xc6ID\xb7wW=455\xedEDW\x10\xd1\xed\x00\x96\x02X\x07\xe03\x00\x04\xe0\xa9\x8a\xa5\xec\xa2Q[[\xdb\x13\xc0,\x00L\x18$\x01x\xa8P(\x1c\xa4\x93\xaf\xab\xab\xdb\xbdP(\xf4\r\xfe\x9dJ\xa5z\x00X\xa7xM\xb7\\.\x0f\xec\x8e\xfa \xa2_\x9a \x0bclD\xc5bv\xc1p]w\x10\x80\r\x92Amg\x8c\x9d\x92H$\x12\x8e\xe3\xf4\x17\x9e\xe3\x0e\xe19R\x00\xb6\x03 "\xfa\x99<\x0f\xe7\xfc"\x8d\xd7\xbc\xb1\xbb\xe9\xa3\xa6\xa6f7\x00\xff2@\x94T\xc5bv\xc1\xf0}\xffd\x00\xcd\x92\xe27;\x8e3 x\x0e\xe0)\xc3\x01}Y__\xdf[\x99k\xb8Fnyw\xd3\x89\xee\x82\x05\x83s~A\xc5j\xba>\xc1\x19\r\xe0+\xd9S\x16\x8b\xc5\x83\x83\xe7\xe5r\xf9\xb0 \xb4k\x0c.\xa9\x99\xefT\x8d\xdc\x0b\xddM/\x00\xd6\x1b\xdeyK2\x99\xac\xaaXN\xd7&9G+F\xe9\xaa4\x0f\x80\xb9\x86\x03r\x8b\xc5\xe2!\x1a\\v\xb5Fvi7\xbb\xac#L\xde\x92\x88\xae\xadXN\x17\x0e\x91\xa8\xfcM1\xa0\x99\xb2L6\x9b\xdd[6\\Ev\x89\xc1\xd3\xd4h\x0e\xf3\xd6n\xe6-M\xd0\xe5\xdf\x8d\x8d\x8d{V\xac\xa7k3\xce\x9f+J\xcf\xe5\xf3\xf9\xfd\x15\x99\x1bL\x9e\xc3\xf7\xfd\xe3u\xf4\n\x80\x82*\xeby\xde\xd0o\xf2]\x93\xc9dU\xb9\\>\x8c16\x96s~\x9eM6\x04\xba\xdc]\xb1\x9c\xae\xf7\n\x1f+J\x9f\x13#+}\xcd`\xec\xd34\xb2\x1f|\x03\x10e(\x80\x05\x00^\x01\xb0\x05\x80+\xed\xe7\xa5\x10\xbd\xdck\x82.\xa5R\xa9\x9f\x1cM<\xcf\x1b\xec\xba\xee\x91\x1d\xc5\x9c\xc9d\xb2\xaaX,\x1e\xe2\xfb\xfe\t\x8c\xb1\x11:\x88\xd4Ah2\x06\xc0j\x00\xf5\x006\xc5\xa1\xbaR\xa9T\x0f\xc7q\xfa\xfb\xbe?\xdc\xf7\xfd\x93;ko\xf5\xf5\xf5\xbd}\xdf\xaf\xf6<o\x88|pC4\xbc\xdc\xe8\x18Y\xe9\x0f\xd4\x85\xd2\xe9t/\x00\x9fj\xc2\xf8%\x89D"\x91\xcf\xe7\xf7\x97+H\xca\xa1/\xd2\x18\xc83\x06\xd9lSS\xd3^!\xc6\x95\xb2\x94L\xef1\xfd.\x93\xc9\xf4\xb1@\x97\xc5\r\r\r{\x10\xd1\x8d\x006\x01 \x99\x9d \xa2\x9b\xe2\x1a#\xe7\xfc|\x00+\x00d5\xeb\xbd\xe3\xfb\xfe\x89\xa6\xdfG\xd5\xa7\xef\xfb\'\x02(+\xcf\xdf\r3F"\xba\x0c\xc0\xb3\x00Z\x0c{\xab\x8e\xb9\xa7\x85\xc2\xf6\x8e\x01\xb0\x08\x80#={9\xf0l\x13\x95\x1f5\xaa\xb7\x1e\xc0_\r\x0b\xa4u\x1e\x82\x88n\xd5\xc8n\x94\x9e\xff\xda\x02\x0b\x8eS\xc2\xe9@\x00\xdc\xb0\xfe\xcc\x10\x88r\xa9\xad\x96\xcf9\x9f`\xf9\xed4\xc3\x9a \xa2\xab\x01\xd4\xd9\xe6&\xa2i\x11\xa9\xa8\xef\x9b\xca\xb8\xca\xba\x19\xb9\x80\xa1\xec5\x92>\x01<\xa6\x9b\xd7\xb2\xb7\t\x00>\x89\xb0\xb7\xad555\xbbE\xd9\x13\x00\x88\x0br\x97\x1c\xbd\xe4\x11Lp\x95\xf2\xc3\xa7\x15\xd7?\xca\xa2\xfc\xa9\x1ar\xfe\xdb\xeaM\x01P\nn\x95\xa8(m7lz\x8d\xc6\xe3\xddg\x90\xf5\x1d\xc79\xd4\xa4\xd4\xc6\xc6\xc6=\x01|fS\xa8\xe7y\xc7X\x08\xf5:\xc3\xba\x0e\x00/\xc2am\xb4\x19dCC\xc3\x1e\x00\x1e1\xfc\xf6s\x01?^W\xf4\xfd5O\x1cG\x9f\x00Vkd\xdeW\xe7lnn\xde\x07\xc0r\xc3\x9cO\x88\xcb\xb4Sb+s\xdd!{\xfa\x08@\xadMw\x89D\xe2\xbf\x95\x1c\x00y\xe9\x87\xf7*/\xf3\'SV\xaa\xd6\xca\x85\xb2\xff\xae1\xe0\xc9\xd2M\xba\xdc\xe2\xc1\xc6i\x98\x80\xbca\xfd\xe5!!<\x19b8%\xf5\x96K\x9e\xe2\xc2\xb6\x0e\x0e\x00[BB\xaf\x89\x1b}2\x80\'\x82-\x91\x9bf\x96i\xbced}\x12\xd1\xa5\x00\xb6\xca\xf0F\x9d\xb3X,\x1e\xa224B\x8e+\xe7\xb8SD\xc9f\xb3{G\xd9S\xe05\x01\xbc\x07`\x8b\xe6\x19\xc98\xf3X\x00\x0b\x01|\x08`\xae\x14F\x0f\x8f\x9a\x95\x8a[\xf2\xacF\xeeN\xc5`6\x9a`\x81F\xe96&\xe0dK6=Px\xb6\x16\x8br\xde\xb3\x18\xf5\x9b\x96\xdf}ND\x97d2\x99>Dt\x99E\xeeu\x83Q\xeeg\xe9\xc0Z\xae\x81Q\xad\xd2\xf3\x15\x9a\xbdF\xd6\xa7t\xf1\xce\x91\x9c\xc6\x15\xc1\xff\x17\n\x85\x83\xd4D\xd8\x14\x1de\x1a\r\xc0\xb6({\x12\xb2\x7f\t\x92\x1c\x9d\x01Gj\x88\xb1\x84\xd1\x9d\xb2\xd2\xfa\xfa\xfa\xde\x00\x9e\x0f\xe3,\x19c\xa7E\x85\x05!L\xc0\x9b!\xfb~F\xc8=lQ\xd0\x92\xb8\x84:\x80\x1d\xe5r\xf909A\xb2\xc8\xce\xd0e\x9f\xa6d\x0c\xc0\xda\xda\xda\xda\x9eJ\xf2\xf5\xa4"3?\xac\xb2f\x83Y\x92~\x16\x8b\xf9X\xd0\x94#\xa2\xdd\xdb\x86\xbd=\xac\xf1\xf8\x05\xe9\xf9\xec(g\x0c`\xa6|\xf18\xe7\xe3T\xa3\x95=\xafv\x84\x84\xd1e\x92\xb7\x1d\xacz\x00\x00_\xea8B\xb9}.\x0c\x16\xd8\xc2)\xe7\xfc\xc20o\x00`\x13\x11]i9\xb8[\x0c\x87\xb6\xd2\xb0G\xa6R+\xbe\xefW[:\x8eFi\xe6^`\x98\xbb\xa9T*}K\x91}Tgl\xcd\xcd\xcd\xfb\xf8\xbe?\\\x97`\xda\xf4\x19\x8c\\.\xb7o\x10I\x00\xbc"\xad\xf7\xb8a\xae\xcdj\x0f\x84\\\x01\x04P\x92;\xc6\xe2\x9cqmmmO"\xba\x05\xc0\\\xce\xf9\xf8\xa8\xa4\xfb\x8d\x960:\xcc\xf7\xfdj\x00\x8f\xcb\x89@\xd0\x9b({S)):\xd2\x92]\xcf\x88\x1aNu\x19\xa0r\x99\xb6\x01`\xbe\xef\x9f\x04\xe0n\x8bq\xffP\xfd\xbd\xe38\x03\x00\xf8\x86u\x17Da\x1f\x84\xec\x87\x9a\x0b\x13\xb9\x11DD\x0b\x1d5C\x11\xf1\xed\x0c\xcb\xb9\xde*\x19\xfaD\xb1\xb7\x8b\r\xf3P\xd0U&y\xc4\xef\xca\xe7HD\xb7E<\xe3\x8e\x17$\x84b\xb6\x1a\x16\xe0\xaa\'\x15\x9d\xec+v"H\xbfnh\x0f\x9a>\xbfP\xbdEH}\xfa\x06\xcb\x1a\x8f\xca\xa1\xc5\x94\xb8\xb5\xb5\xb5\xb5\xb9\xae;H\xf3\xfb9\xa6=:\x8e\xd3?\xac0!\xed\xf1zM\xe8\xfb\xc20\xf7\x0b\x06\x0e\x95\x0c\xf2_\x89/\x05\x1a\xa2\xeaS\x81\x12_\x04^\xba\xae\xaenw\x81yM{{B)V\x0c\x06\xd0$=\x7f^v\x12\x00\x1e\x88\xbb\xa7X\x83s\xfe#\x1b\xf8\x17)\xffZ\x00\x8f\x10\xd1\xe5\xf9|~?\xdb|"|\xb4Fyy\xf1\x82+\x0c\xb2y\x13\x06\xe1\x9c\x9f)\xb2\xbd\x7f\x04\xa1G\x97]\x06\x17I\xf5\xba\xc2\x18r\x06\xf9?j\xd6\x1b\x17\xb5\x86n\xc2\xba\x00\x98\xee\x82\x08\xcf6]\xa5~|\xdf\xafN&\x93U\x82\xcei\x89\xaaOi\xcek\xd4n0\x00\x0f\x99\x1c\x90\xe7yGK\xef{:\x80/\xa5\xe7\xaf\xc8\xa1Y\x86\x08Q\xf4\xd7\xde\x12\xa5\t\x04\xd7\x99\xc2h\x08,\xb8\xd9\x02\x0b\x86+\xe1\xf4PK8\xbd_7\xbf\xa08v\x00(\xc9^\xdb\x82\x917k\xf6\xf8\x0b\x1bt\xd1\xe8\xe8\r\xc3\xdcw*\x10f\x90\xe5}\x16[\xce`\x99\x12\xee\xcf\x89\x08\xb3\x86[\x9cC\x93X\xb7\xa5\xa5\xa5\xe5\x00\x11z=\x9b1\x89~\x81\xc7\xe4\x10\r`^*\x95\xea\x11\xe3\x8c\x87uF-\xf5;\x1d\xadfh:\x97\xb6\x19^~\x9d\xe6@f\x99n\xb0\xee\xa3/\x01;^S3\xd1\xd6\xd6\xd6\x03-^\x7f\x95f\x8eOL\xd9\xb2FG#M\xdc\xa8Z\x9d\x01\xb0\xc4\x84\xdf\\\xd7=J\xa7\xb3B\xa1\xd0W)\xd3m\x0b\xb2\xd9\x10\x98\xb5\xceb\xe8\x0fHrw\x99*A2\xc4\x00\xf0\xa6Rn\xfd\x88s~z\xcc3^\xdbY\xde\xf2i\xc3\x02\xb9L&\xd3\xa7\x1d\xb0`BT\xd0/:\x92\x9a\r\xeb\xd7\x18\xbc\xf1\xef\xc5\xf3\xe7b\xd0>3\x95=\x8e\xb7\xec\xf1\xbc\x18\xb5\xfby\x1aO\xee\x1adW[\xce`\x8e\xe2\x10nkO\x12%\xb1\x07\xc7I\xdfn\xed\xc8d2}Z[[\x0f\x94\x8d\xdf\xa2+\x02\xb0\x9es>A\xf5\x92Q\xa0_X\x07W\xa4!\x88i\x13\xa1>\xa7\xaba\x01\x11]g\xa1_\xc6h\x8cx~p\xa3U\x8e\x8f\x88&Y<\xff\x95Q\x1a=t\x1d\xea\xa2\xe4\xcau\x00_\xe68u8Q9\xb0\x8bu\xfa\x12PF\xae\xf6dd\\m\xa9\x18ia\x96(|\xbc+\xbd\xfbUa\xc5\x0b\x00\xaf\x02\x98MD\x93\xe4/\x18\xdaq\xc6\x9d\xd3\xe1\x0f\xe0~K]z@;`\xc1\xa8\xa8\xb0@\x84\xa8\x8f\r\xebo\xd0\xd0B*w\xfa\x85\x89k\xd3`\x9e\x13\x82\xae\x1e\xdbeP\xb3\xeb\x90da\xa9F\xd6Xo7uE\xa9\x8d\xc9DtsD\xb6b\x9aa\xbeY:/-\x1b\xab\nG\xe24@\xab\xbd\xbca\xfa\x8b=D\xa6\xd7\xae\xbatT\x00o\xcb\xae\x89h\xb2\xe5\x05\'*\xf3>\x16V\x9d\xb0x\xc1|*\x95\xea\xe1\xba\xee\x11j\x93\x84\n]T\xe3\x11\tD1J\x89\xd4q\x9c\x016\x8f\x14%$\x02H\xcb\xe4\xb6\xa5\xb1\xe2+\x1d[!G\r\x00\xb9\x80\xf2\x12\x18\x96\xe2B\x0c\xb5U\x0f\xc0\xec8\xfak\xd7 \xa2\x9b,\x1e\xe6\xa4\xb8\xf3\t\xfa\xa5\x14%\x1b-\x95J\xfd\x00\xec0\xc8\x16\xe4\x1b\xec8N\x7f\xe5y\x19\xc0JYF\xccg\x82$/\x89\xef\xe5\x9d\x10l5[\xa3\xa3\xa9\x06\xd9O4\x97\xb2\xc62\xf7\\MC\xf3\x10\x99\xaeR+M---\x07\xa8\xbd\x94\xa6Re@\x9a\x07l\x00\x00\x92\x0b\n\x9c\xf3s-{\xbb7\xecl\x05\xf7\xb9*\xae\xfeb\x0f\x91U}\x1a7\xd3\x0bIz\xc6[<\xe0\xb5\xcaKn\xb2\xbc\xe0\x1b\xca\x81\xcf\xd3Q(\x8a\xcc\xc2\x08\xa0\xfeE\x00\x8b\xe2@\x17\x00k\x0c\xf2\xcf\xaa]\xe2!\xbd\x9a\xbf\xd3\x18\xe5g\x8a\xccobt\x11\x8d\x97=\x19\x11M\x97/&\x11MW\xe6\xfa\x95E/w\x86\x9c\xeb8\x00\r")j\xecL\xe8\x177s>\xbf\x9da\xfcn\x1b\x15\xc1\x18\x1bCDSM4\x83Rw\xbf\x801v\n\x11\xfd\xd6\xc4\xedI\x87r[\xc8|\xcdD4E$O&B}\xa5!t\x99:\xb2\x9b8\xe7\xe73\xc6N\x03\x904E\n\x99G\xf5}\xbf\xdaq\x9cC\x89h\x9a\xda)o\xe8\xe2\xbf\xcf2\xdfb\xa1\xcf\xc9\xea%\xd7yg"\xba\xc32\xd7[\xe9t\xba\x97\xa6\xc3\xfe\\\x00\xaf\x068\x94\x88&Y:\xfcWv\x16E\xf4NggU6\x8eL\xb3\xce\x83&\x9a(\xe4w\xf5D4\x9d\x88\xa6\x00\xb8\xc7\xd6\t.J\xa9\x0b\x83,\x93\x88\xae\xb70\x00#t\x04u\xcc\xbd\xe5\xdb\xd9\xc7Y\xa3\xa3e\xd4N\xa3\x88s\xcd\x0e\xab\x00\x99\xf4*>\xf3X\n\xe0e\xa5\x0c\xb9\x9516\xd2\xd4\xe1o\xd2_{\x08\xf5\xd1\x96\x90s]\x07\x8c}VD\xe5\xcd\x13\xde\xa81\x82\xac#\xfe4\rb\x1e\xd0\x1a\xcf\xf3\x06+\xde\xcfD\xa8\xaf\xb7\xf4\x0f\x14#\xae7\xcbu\xdd\xa3L\r\r\x96\xe6\xd9\xbbL\x8e\xc0\xf4a\x9ca\xae"\x11M2\x9d\x8d\xad\xd9"d\x7f\xcbr\xb9\xdc\xbe!\x1d\xfe\xeb;\xcb[\xd6\x18\x16\xc8v$\xab\xf2}\x7f\xb8\xad\x1bF(oJX\xc6)\xc9\x7f\x10|\x9ea\x0bkj\xb6\xaa\xfb\xa3\x04\x9c\xf3\x0b,\xd0\xe5"\x8b\xae\x16D0\x88\x1fK\xf23"\xees3\xe7\xfc\xec\x10}\x1eo*m\xaa\xd8\xd9\xf4\xc9\x88\xf2.\xf7\xc40\xca\xb5r\x1b_{\xf5\x17y\x08\xca\xa4]\x1fzE\xcc\xf4/\x05\x90\xd1t\xc7<\xa2\x82c\x91I\xbf\xad\xb9\xa5\x1b\x88\xe8\xa7\x9a\xda\xec5\x00\xeau\xc6\x01\xe0u"\x9aj\xe1\n\xd7\xc5m\xa9\x93\x9ai\x17\xa9\x06"\xde\xe9\x0f:\xc0OD?\x01\x90\xd65p\x00XCDW\x9a**\xba^S\x00\xff4t\x1c-e\x8c\x8d\x8dy>S-\xe5\xcd\x8f\x00\xcc\x0f8\xdf\xce\xd0\xdf\xff\xd4H\xa7\xd3\xbd\x18c#9\xe7gz\x9e74\xec\x10<\xcf;\x96s~6cl\xb4\xfaG\x17\x0c\xf2G3\xc6\xbe\xc7\x18\x1b\xeby\xde\xb1r\x17xW\x8d\\.\xb7/cl4\xe7\xfc,\xdf\xf7\xab\xa3\xacY*\x95\xfa1\xc6\xc6p\xce\xcf\xf2<ohG\xa2\x91\xeb\xba\x838\xe7g0\xc6N-\x97\xcb\x03\xa3\x1a\xb6\xa5\xe2w8c\xecT\xf1>\xc3L_c\xee\xaa\xf1\x1f9\xec5\xd4\xfe\r\x9d\x05\x00\x00\x00\x00IEND\xaeB`\x82'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

like a 2-by-2 square might be better

if header_name != 'attachment':
qs[header_name] = part.text
else:
qs[header_name] = part.content

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

store qs.get('message') as a variable before you even do this conditional. Will improve readability significantly. Also, get returns None by default

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it raised a KeyError if you didn't give it a second argument.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh I stand corrected

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed the , None's, but rejected on the first part. I think it's readable enough.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still disagree!

content_type='application/json'
)

img_path = tmpdir.join('pushover.png')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this automatically get cleaned up? Usually this sort of thing works with a context manager (but that code wasn't 2/3 compatible :/)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check but I'm 99% sure pytest handles the cleanup.

req_body = getattr(request, 'body', None)
qs = parse_qs(req_body)
qs = {k: v[0] for k, v in qs.items()}
if getattr(request, 'headers', None).get('content-type', None) == 'application/x-www-form-urlencoded':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this breaks if the headers isn't in request and you try doing None.get

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are you even doing getattr? this is terrifying

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea. I was doing it before...

If there's no headers the text should fail anyway. I'll just switch to normal access and let it blow up if something goes that bad.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I figured out why I did this. I want the tests to not fail if the attr isn't there but just to fail the test based on the next criteria. I've changed this particular line to getattr(..., {}).get(...) it's the only place I'm chaining off the getattr and not just comparing.

multipart_data = decoder.MultipartDecoder.from_response(request)
qs = {}
for part in multipart_data.parts:
header_name = part.headers[b'Content-Disposition'].decode('utf-8').split(';')[1].split('=')[1].strip('"')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hate this line. Make this a function that's documented exactly what it's doing, and split it on to multiple lines in that function with variable names that make it obvious what's going on

@@ -6,4 +6,5 @@ Authors
Contributors
------------

- Arno Hautala <arno@alum.wpiedu>: image attachment support
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you throw out his PR? he doesn't get github cred this way 👎

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did, but I'm not gonna squash the commits so he's still in there.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good :)

INSTALL_REQUIRES = ['requests']
INSTALL_REQUIRES = ['requests', 'six']
if sys.version_info < (3, 4): # pathlib added to stdlib in 3.4
INSTALL_REQUIRES.append('pathlib2')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait they added a library that doesn't have the same name as the expected one?

Like if you install functools32, it imports as functools

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's

try:
    from pathlib import Path
except ImportError:
    from pathlib2 import Path

Yes it's kinda weird but pathlib2 is a replacement for pathlib that's also on PyPI so I guess they had to? Nothing that couldn't be solved by import ... as ... though

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so its fine that you're importing directly and erasing the module from the logic.

try:
from urllib.parse import urljoin
from pathlib import Path
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JK not python3 only.

attachment = {'attachment': f}
return self._generic_post('messages.json', payload=payload, session=session, files=attachment)
# otherwise, assume it's a file-like (no good way to test that in both Python 2 and 3...)
else:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well you could always test it has the right functions, depending on how self._generic_post (and actually the requests library) handles the stuff in here. Does it need .read() or just an __iter__ function? Because you could actually use hasattr to check that else raise a type error. Raising type errors for bad input is much better for testing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OR

use a try/except on self._generic_post and hope to god that it can handle whatever goes in, else it throws an exception, which you can catch and reraise with extra info

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sorta like that--it's elegant. But I'd worry about swallowing other exceptions and always laying the blame on the file you passed in.

Checking if the duck quacks is like pretty good, but then I'm relying on requests not changing from using read() (or whatever) to doing something else. Either way, requests doesn't specify how it does it, just that it wants a file open in binary mode.

# the Pushover logo
TEST_IMAGE_BYTES = b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\xa6\x00\x00\x00$\x08\x06\x00\x00\x00\x03\xf0A\xa4\x00\x00\x00\x06bKGD\x00\x00\x00\x00\x00\x00\xf9C\xbb\x7f\x00\x00\x00\tpHYs\x00\x00\x0b\x13\x00\x00\x0b\x13\x01\x00\x9a\x9c\x18\x00\x00\x00\x07tIME\x07\xdf\x0b\x03\x022\x05\xd4G\xee5\x00\x00\x0c\xddIDATx\xda\xed\\i\x94T\xc5\x15\xee\x11\x10\x15wQ\x82(\x1a\\\x00G\x05\x05\x82h\x88\xeb\t\xc9qA\r.\xa0&\xc4@\\B\\\x8e!\x9aDc+\xb2\x88\x1e\x05\x17\xa2\xacQ\xc0\xe8\xa8\x1c\xa3\x1eD#\xd2\x12\x17\x90I\xa2\x07;\x8a\x13ddd\x1c;=m\xcft\xf7\xeb\xf7^\xd5\xfd&?R\xef\x9c\xa2\xac\xaa\xf7\xde,\x98\xc9\xe9\xfa\x07\xefvU\xbd[\xb7\xee\xfd\xeew\xef\x9b\xaaD\'\x0c\xd7u\x07\xf5\xec\xd9sHUU\xd5\xc1\x89D\xe2\xe0D"\xb1["\x91\xc8\xb6\xb5\xb55\xe6\xf3\xf9\xb7\xfa\xf6\xed[LTFet\xf5\xc8f\xb3{\x13\xd1d\x00\xcf\x00hl\xb3\x0c\x00\x1e\x80\xd5\x8c\xb1Q\x15\xcdUF\x97\x0c\xc6\xd8H\x00\xcb\x00\x14%\xc3s\x01l\x04\xf0"\x80\'\x00\xac\x02\xf0\xbe\xc1Hg\xfd?\xea%\x9dN\xf7r]\xf7\x88\x8a\x85\xec\xe2\xe1y\xde`\x00\xcfI\x06V\x06\xb0\x84s~fCC\xc3\x1e\x86\xf0~$\x80?\xab\xc6ID\xb7wW=455\xedEDW\x10\xd1\xed\x00\x96\x02X\x07\xe03\x00\x04\xe0\xa9\x8a\xa5\xec\xa2Q[[\xdb\x13\xc0,\x00L\x18$\x01x\xa8P(\x1c\xa4\x93\xaf\xab\xab\xdb\xbdP(\xf4\r\xfe\x9dJ\xa5z\x00X\xa7xM\xb7\\.\x0f\xec\x8e\xfa \xa2_\x9a \x0bclD\xc5bv\xc1p]w\x10\x80\r\x92Amg\x8c\x9d\x92H$\x12\x8e\xe3\xf4\x17\x9e\xe3\x0e\xe19R\x00\xb6\x03 "\xfa\x99<\x0f\xe7\xfc"\x8d\xd7\xbc\xb1\xbb\xe9\xa3\xa6\xa6f7\x00\xff2@\x94T\xc5bv\xc1\xf0}\xffd\x00\xcd\x92\xe27;\x8e3 x\x0e\xe0)\xc3\x01}Y__\xdf[\x99k\xb8Fnyw\xd3\x89\xee\x82\x05\x83s~A\xc5j\xba>\xc1\x19\r\xe0+\xd9S\x16\x8b\xc5\x83\x83\xe7\xe5r\xf9\xb0 \xb4k\x0c.\xa9\x99\xefT\x8d\xdc\x0b\xddM/\x00\xd6\x1b\xdeyK2\x99\xac\xaaXN\xd7&9G+F\xe9\xaa4\x0f\x80\xb9\x86\x03r\x8b\xc5\xe2!\x1a\\v\xb5Fvi7\xbb\xac#L\xde\x92\x88\xae\xadXN\x17\x0e\x91\xa8\xfcM1\xa0\x99\xb2L6\x9b\xdd[6\\Ev\x89\xc1\xd3\xd4h\x0e\xf3\xd6n\xe6-M\xd0\xe5\xdf\x8d\x8d\x8d{V\xac\xa7k3\xce\x9f+J\xcf\xe5\xf3\xf9\xfd\x15\x99\x1bL\x9e\xc3\xf7\xfd\xe3u\xf4\n\x80\x82*\xeby\xde\xd0o\xf2]\x93\xc9dU\xb9\\>\x8c16\x96s~\x9eM6\x04\xba\xdc]\xb1\x9c\xae\xf7\n\x1f+J\x9f\x13#+}\xcd`\xec\xd34\xb2\x1f|\x03\x10e(\x80\x05\x00^\x01\xb0\x05\x80+\xed\xe7\xa5\x10\xbd\xdck\x82.\xa5R\xa9\x9f\x1cM<\xcf\x1b\xec\xba\xee\x91\x1d\xc5\x9c\xc9d\xb2\xaaX,\x1e\xe2\xfb\xfe\t\x8c\xb1\x11:\x88\xd4Ah2\x06\xc0j\x00\xf5\x006\xc5\xa1\xbaR\xa9T\x0f\xc7q\xfa\xfb\xbe?\xdc\xf7\xfd\x93;ko\xf5\xf5\xf5\xbd}\xdf\xaf\xf6<o\x88|pC4\xbc\xdc\xe8\x18Y\xe9\x0f\xd4\x85\xd2\xe9t/\x00\x9fj\xc2\xf8%\x89D"\x91\xcf\xe7\xf7\x97+H\xca\xa1/\xd2\x18\xc83\x06\xd9lSS\xd3^!\xc6\x95\xb2\x94L\xef1\xfd.\x93\xc9\xf4\xb1@\x97\xc5\r\r\r{\x10\xd1\x8d\x006\x01 \x99\x9d \xa2\x9b\xe2\x1a#\xe7\xfc|\x00+\x00d5\xeb\xbd\xe3\xfb\xfe\x89\xa6\xdfG\xd5\xa7\xef\xfb\'\x02(+\xcf\xdf\r3F"\xba\x0c\xc0\xb3\x00Z\x0c{\xab\x8e\xb9\xa7\x85\xc2\xf6\x8e\x01\xb0\x08\x80#={9\xf0l\x13\x95\x1f5\xaa\xb7\x1e\xc0_\r\x0b\xa4u\x1e\x82\x88n\xd5\xc8n\x94\x9e\xff\xda\x02\x0b\x8eS\xc2\xe9@\x00\xdc\xb0\xfe\xcc\x10\x88r\xa9\xad\x96\xcf9\x9f`\xf9\xed4\xc3\x9a \xa2\xab\x01\xd4\xd9\xe6&\xa2i\x11\xa9\xa8\xef\x9b\xca\xb8\xca\xba\x19\xb9\x80\xa1\xec5\x92>\x01<\xa6\x9b\xd7\xb2\xb7\t\x00>\x89\xb0\xb7\xad555\xbbE\xd9\x13\x00\x88\x0br\x97\x1c\xbd\xe4\x11Lp\x95\xf2\xc3\xa7\x15\xd7?\xca\xa2\xfc\xa9\x1ar\xfe\xdb\xeaM\x01P\nn\x95\xa8(m7lz\x8d\xc6\xe3\xddg\x90\xf5\x1d\xc79\xd4\xa4\xd4\xc6\xc6\xc6=\x01|fS\xa8\xe7y\xc7X\x08\xf5:\xc3\xba\x0e\x00/\xc2am\xb4\x19dCC\xc3\x1e\x00\x1e1\xfc\xf6s\x01?^W\xf4\xfd5O\x1cG\x9f\x00Vkd\xdeW\xe7lnn\xde\x07\xc0r\xc3\x9cO\x88\xcb\xb4Sb+s\xdd!{\xfa\x08@\xadMw\x89D\xe2\xbf\x95\x1c\x00y\xe9\x87\xf7*/\xf3\'SV\xaa\xd6\xca\x85\xb2\xff\xae1\xe0\xc9\xd2M\xba\xdc\xe2\xc1\xc6i\x98\x80\xbca\xfd\xe5!!<\x19b8%\xf5\x96K\x9e\xe2\xc2\xb6\x0e\x0e\x00[BB\xaf\x89\x1b}2\x80\'\x82-\x91\x9bf\x96i\xbced}\x12\xd1\xa5\x00\xb6\xca\xf0F\x9d\xb3X,\x1e\xa224B\x8e+\xe7\xb8SD\xc9f\xb3{G\xd9S\xe05\x01\xbc\x07`\x8b\xe6\x19\xc98\xf3X\x00\x0b\x01|\x08`\xae\x14F\x0f\x8f\x9a\x95\x8a[\xf2\xacF\xeeN\xc5`6\x9a`\x81F\xe96&\xe0dK6=Px\xb6\x16\x8br\xde\xb3\x18\xf5\x9b\x96\xdf}ND\x97d2\x99>Dt\x99E\xeeu\x83Q\xeeg\xe9\xc0Z\xae\x81Q\xad\xd2\xf3\x15\x9a\xbdF\xd6\xa7t\xf1\xce\x91\x9c\xc6\x15\xc1\xff\x17\n\x85\x83\xd4D\xd8\x14\x1de\x1a\r\xc0\xb6({\x12\xb2\x7f\t\x92\x1c\x9d\x01Gj\x88\xb1\x84\xd1\x9d\xb2\xd2\xfa\xfa\xfa\xde\x00\x9e\x0f\xe3,\x19c\xa7E\x85\x05!L\xc0\x9b!\xfb~F\xc8=lQ\xd0\x92\xb8\x84:\x80\x1d\xe5r\xf909A\xb2\xc8\xce\xd0e\x9f\xa6d\x0c\xc0\xda\xda\xda\xda\x9eJ\xf2\xf5\xa4"3?\xac\xb2f\x83Y\x92~\x16\x8b\xf9X\xd0\x94#\xa2\xdd\xdb\x86\xbd=\xac\xf1\xf8\x05\xe9\xf9\xec(g\x0c`\xa6|\xf18\xe7\xe3T\xa3\x95=\xafv\x84\x84\xd1e\x92\xb7\x1d\xacz\x00\x00_\xea8B\xb9}.\x0c\x16\xd8\xc2)\xe7\xfc\xc20o\x00`\x13\x11]i9\xb8[\x0c\x87\xb6\xd2\xb0G\xa6R+\xbe\xefW[:\x8eFi\xe6^`\x98\xbb\xa9T*}K\x91}Tgl\xcd\xcd\xcd\xfb\xf8\xbe?\\\x97`\xda\xf4\x19\x8c\\.\xb7o\x10I\x00\xbc"\xad\xf7\xb8a\xae\xcdj\x0f\x84\\\x01\x04P\x92;\xc6\xe2\x9cqmmmO"\xba\x05\xc0\\\xce\xf9\xf8\xa8\xa4\xfb\x8d\x960:\xcc\xf7\xfdj\x00\x8f\xcb\x89@\xd0\x9b({S)):\xd2\x92]\xcf\x88\x1aNu\x19\xa0r\x99\xb6\x01`\xbe\xef\x9f\x04\xe0n\x8bq\xffP\xfd\xbd\xe38\x03\x00\xf8\x86u\x17Da\x1f\x84\xec\x87\x9a\x0b\x13\xb9\x11DD\x0b\x1d5C\x11\xf1\xed\x0c\xcb\xb9\xde*\x19\xfaD\xb1\xb7\x8b\r\xf3P\xd0U&y\xc4\xef\xca\xe7HD\xb7E<\xe3\x8e\x17$\x84b\xb6\x1a\x16\xe0\xaa\'\x15\x9d\xec+v"H\xbfnh\x0f\x9a>\xbfP\xbdEH}\xfa\x06\xcb\x1a\x8f\xca\xa1\xc5\x94\xb8\xb5\xb5\xb5\xb5\xb9\xae;H\xf3\xfb9\xa6=:\x8e\xd3?\xac0!\xed\xf1zM\xe8\xfb\xc20\xf7\x0b\x06\x0e\x95\x0c\xf2_\x89/\x05\x1a\xa2\xeaS\x81\x12_\x04^\xba\xae\xaenw\x81yM{{B)V\x0c\x06\xd0$=\x7f^v\x12\x00\x1e\x88\xbb\xa7X\x83s\xfe#\x1b\xf8\x17)\xffZ\x00\x8f\x10\xd1\xe5\xf9|~?\xdb|"|\xb4Fyy\xf1\x82+\x0c\xb2y\x13\x06\xe1\x9c\x9f)\xb2\xbd\x7f\x04\xa1G\x97]\x06\x17I\xf5\xba\xc2\x18r\x06\xf9?j\xd6\x1b\x17\xb5\x86n\xc2\xba\x00\x98\xee\x82\x08\xcf6]\xa5~|\xdf\xafN&\x93U\x82\xcei\x89\xaaOi\xcek\xd4n0\x00\x0f\x99\x1c\x90\xe7yGK\xef{:\x80/\xa5\xe7\xaf\xc8\xa1Y\x86\x08Q\xf4\xd7\xde\x12\xa5\t\x04\xd7\x99\xc2h\x08,\xb8\xd9\x02\x0b\x86+\xe1\xf4PK8\xbd_7\xbf\xa08v\x00(\xc9^\xdb\x82\x917k\xf6\xf8\x0b\x1bt\xd1\xe8\xe8\r\xc3\xdcw*\x10f\x90\xe5}\x16[\xce`\x99\x12\xee\xcf\x89\x08\xb3\x86[\x9cC\x93X\xb7\xa5\xa5\xa5\xe5\x00\x11z=\x9b1\x89~\x81\xc7\xe4\x10\r`^*\x95\xea\x11\xe3\x8c\x87uF-\xf5;\x1d\xadfh:\x97\xb6\x19^~\x9d\xe6@f\x99n\xb0\xee\xa3/\x01;^S3\xd1\xd6\xd6\xd6\x03-^\x7f\x95f\x8eOL\xd9\xb2FG#M\xdc\xa8Z\x9d\x01\xb0\xc4\x84\xdf\\\xd7=J\xa7\xb3B\xa1\xd0W)\xd3m\x0b\xb2\xd9\x10\x98\xb5\xceb\xe8\x0fHrw\x99*A2\xc4\x00\xf0\xa6Rn\xfd\x88s~z\xcc3^\xdbY\xde\xf2i\xc3\x02\xb9L&\xd3\xa7\x1d\xb0`BT\xd0/:\x92\x9a\r\xeb\xd7\x18\xbc\xf1\xef\xc5\xf3\xe7b\xd0>3\x95=\x8e\xb7\xec\xf1\xbc\x18\xb5\xfby\x1aO\xee\x1adW[\xce`\x8e\xe2\x10nkO\x12%\xb1\x07\xc7I\xdfn\xed\xc8d2}Z[[\x0f\x94\x8d\xdf\xa2+\x02\xb0\x9es>A\xf5\x92Q\xa0_X\x07W\xa4!\x88i\x13\xa1>\xa7\xaba\x01\x11]g\xa1_\xc6h\x8cx~p\xa3U\x8e\x8f\x88&Y<\xff\x95Q\x1a=t\x1d\xea\xa2\xe4\xcau\x00_\xe68u8Q9\xb0\x8bu\xfa\x12PF\xae\xf6dd\\m\xa9\x18ia\x96(|\xbc+\xbd\xfbUa\xc5\x0b\x00\xaf\x02\x98MD\x93\xe4/\x18\xdaq\xc6\x9d\xd3\xe1\x0f\xe0~K]z@;`\xc1\xa8\xa8\xb0@\x84\xa8\x8f\r\xebo\xd0\xd0B*w\xfa\x85\x89k\xd3`\x9e\x13\x82\xae\x1e\xdbeP\xb3\xeb\x90da\xa9F\xd6Xo7uE\xa9\x8d\xc9DtsD\xb6b\x9aa\xbeY:/-\x1b\xab\nG\xe24@\xab\xbd\xbca\xfa\x8b=D\xa6\xd7\xae\xbatT\x00o\xcb\xae\x89h\xb2\xe5\x05\'*\xf3>\x16V\x9d\xb0x\xc1|*\x95\xea\xe1\xba\xee\x11j\x93\x84\n]T\xe3\x11\tD1J\x89\xd4q\x9c\x016\x8f\x14%$\x02H\xcb\xe4\xb6\xa5\xb1\xe2+\x1d[!G\r\x00\xb9\x80\xf2\x12\x18\x96\xe2B\x0c\xb5U\x0f\xc0\xec8\xfak\xd7 \xa2\x9b,\x1e\xe6\xa4\xb8\xf3\t\xfa\xa5\x14%\x1b-\x95J\xfd\x00\xec0\xc8\x16\xe4\x1b\xec8N\x7f\xe5y\x19\xc0JYF\xccg\x82$/\x89\xef\xe5\x9d\x10l5[\xa3\xa3\xa9\x06\xd9O4\x97\xb2\xc62\xf7\\MC\xf3\x10\x99\xaeR+M---\x07\xa8\xbd\x94\xa6Re@\x9a\x07l\x00\x00\x92\x0b\n\x9c\xf3s-{\xbb7\xecl\x05\xf7\xb9*\xae\xfeb\x0f\x91U}\x1a7\xd3\x0bIz\xc6[<\xe0\xb5\xcaKn\xb2\xbc\xe0\x1b\xca\x81\xcf\xd3Q(\x8a\xcc\xc2\x08\xa0\xfeE\x00\x8b\xe2@\x17\x00k\x0c\xf2\xcf\xaa]\xe2!\xbd\x9a\xbf\xd3\x18\xe5g\x8a\xccobt\x11\x8d\x97=\x19\x11M\x97/&\x11MW\xe6\xfa\x95E/w\x86\x9c\xeb8\x00\r")j\xecL\xe8\x177s>\xbf\x9da\xfcn\x1b\x15\xc1\x18\x1bCDSM4\x83Rw\xbf\x801v\n\x11\xfd\xd6\xc4\xedI\x87r[\xc8|\xcdD4E$O&B}\xa5!t\x99:\xb2\x9b8\xe7\xe73\xc6N\x03\x904E\n\x99G\xf5}\xbf\xdaq\x9cC\x89h\x9a\xda)o\xe8\xe2\xbf\xcf2\xdfb\xa1\xcf\xc9\xea%\xd7yg"\xba\xc32\xd7[\xe9t\xba\x97\xa6\xc3\xfe\\\x00\xaf\x068\x94\x88&Y:\xfcWv\x16E\xf4NggU6\x8eL\xb3\xce\x83&\x9a(\xe4w\xf5D4\x9d\x88\xa6\x00\xb8\xc7\xd6\t.J\xa9\x0b\x83,\x93\x88\xae\xb70\x00#t\x04u\xcc\xbd\xe5\xdb\xd9\xc7Y\xa3\xa3e\xd4N\xa3\x88s\xcd\x0e\xab\x00\x99\xf4*>\xf3X\n\xe0e\xa5\x0c\xb9\x9516\xd2\xd4\xe1o\xd2_{\x08\xf5\xd1\x96\x90s]\x07\x8c}VD\xe5\xcd\x13\xde\xa81\x82\xac#\xfe4\rb\x1e\xd0\x1a\xcf\xf3\x06+\xde\xcfD\xa8\xaf\xb7\xf4\x0f\x14#\xae7\xcbu\xdd\xa3L\r\r\x96\xe6\xd9\xbbL\x8e\xc0\xf4a\x9ca\xae"\x11M2\x9d\x8d\xad\xd9"d\x7f\xcbr\xb9\xdc\xbe!\x1d\xfe\xeb;\xcb[\xd6\x18\x16\xc8v$\xab\xf2}\x7f\xb8\xad\x1bF(oJX\xc6)\xc9\x7f\x10|\x9ea\x0bkj\xb6\xaa\xfb\xa3\x04\x9c\xf3\x0b,\xd0\xe5"\x8b\xae\x16D0\x88\x1fK\xf23"\xees3\xe7\xfc\xec\x10}\x1eo*m\xaa\xd8\xd9\xf4\xc9\x88\xf2.\xf7\xc40\xca\xb5r\x1b_{\xf5\x17y\x08\xca\xa4]\x1fzE\xcc\xf4/\x05\x90\xd1t\xc7<\xa2\x82c\x91I\xbf\xad\xb9\xa5\x1b\x88\xe8\xa7\x9a\xda\xec5\x00\xeau\xc6\x01\xe0u"\x9aj\xe1\n\xd7\xc5m\xa9\x93\x9ai\x17\xa9\x06"\xde\xe9\x0f:\xc0OD?\x01\x90\xd65p\x00XCDW\x9a**\xba^S\x00\xff4t\x1c-e\x8c\x8d\x8dy>S-\xe5\xcd\x8f\x00\xcc\x0f8\xdf\xce\xd0\xdf\xff\xd4H\xa7\xd3\xbd\x18c#9\xe7gz\x9e74\xec\x10<\xcf;\x96s~6cl\xb4\xfaG\x17\x0c\xf2G3\xc6\xbe\xc7\x18\x1b\xeby\xde\xb1r\x17xW\x8d\\.\xb7/cl4\xe7\xfc,\xdf\xf7\xab\xa3\xacY*\x95\xfa1\xc6\xc6p\xce\xcf\xf2<ohG\xa2\x91\xeb\xba\x838\xe7g0\xc6N-\x97\xcb\x03\xa3\x1a\xb6\xa5\xe2w8c\xecT\xf1>\xc3L_c\xee\xaa\xf1\x1f9\xec5\xd4\xfe\r\x9d\x05\x00\x00\x00\x00IEND\xaeB`\x82'
# a single black pixel
TEST_IMAGE_BYTES = b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00\x01\x00\x00\x00\x01\x01\x00\x00\x00\x007n\xf9$\x00\x00\x00\nIDATx\x9cc`\x00\x00\x00\x02\x00\x01H\xaf\xa4q\x00\x00\x00\x00IEND\xaeB`\x82'
Copy link
Contributor

@cthoyt cthoyt Feb 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line is too long. can byte strings be written on multiple lines?

TEST_IMAGE_BYTES = (
    b'\x89PNG\r\n\x1a\n\x00\x00\x00\rIHDR\x00\x00\x00'  
  '\x01\x00\x00\x00\x01\x01\x00\x00\x00\x007n\xf9$\'
'x00\x00\x00\nIDATx\x9cc`\x00\x00\x00\x02\x00\x01H'
'\xaf\xa4q\x00\x00\x00\x00IEND\xaeB`\x82'
 )

?

Copy link
Contributor

@cthoyt cthoyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple minor name changes, but I still think you should store those gets into variables (even if this means more lines) so the if statements operating over them are more self-descriptive

@@ -21,7 +30,7 @@ def generic_callback(request):
qs = parse_qs(req_body)
qs = {k: v[0] for k, v in qs.items()}

if qs.get('payload-test', None) is not None:
if qs.get('payload-test') is not None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since you get it multiple times, just store it as a variable before the conditional

@@ -36,7 +45,7 @@ def messages_callback(request):
}
headers = {'X-Request-Id': TEST_REQUEST_ID}

if getattr(request, 'headers', None).get('content-type', None) == 'application/x-www-form-urlencoded':
if getattr(request, 'headers', {}).get('content-type') == 'application/x-www-form-urlencoded':
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, I'd rather see this stored as a variable before the conditional and that you explicitly state the logic behind these checks

if header_name != 'attachment':
qs[header_name] = part.text
else:
qs[header_name] = part.content

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still disagree!

@@ -95,7 +104,7 @@ def sounds_callback(request):
qs = parse_qs(req_body)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need better variable name than qs

@@ -95,7 +104,7 @@ def sounds_callback(request):
qs = parse_qs(req_body)
qs = {k: v[0] for k, v in qs.items()}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need better variable names than k and v

@scolby33 scolby33 merged commit 0dd9424 into develop Apr 7, 2018
@scolby33 scolby33 deleted the feature/image_attachment_support branch April 7, 2018 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants