aioaws icon indicating copy to clipboard operation
aioaws copied to clipboard

add basic SQS consumer functionality

Open adriangb opened this issue 2 years ago • 5 comments

Initial draft for implementation and testing plan. @samuelcolvin I'd appreciate an initial review and then I can dig deeper into testing edge cases, getting the queue URL, etc.

adriangb avatar Aug 05 '22 07:08 adriangb

Codecov Report

Merging #33 (a6a3148) into main (64f8cfe) will decrease coverage by 1.44%. The diff coverage is 92.22%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #33      +/-   ##
==========================================
- Coverage   97.39%   95.94%   -1.45%     
==========================================
  Files           9       10       +1     
  Lines         537      617      +80     
  Branches       91      100       +9     
==========================================
+ Hits          523      592      +69     
- Misses          9       17       +8     
- Partials        5        8       +3     
Impacted Files Coverage Δ
aioaws/sqs.py 89.39% <89.39%> (ø)
aioaws/core.py 98.34% <100.00%> (-1.66%) :arrow_down:
aioaws/s3.py 97.41% <0.00%> (-1.73%) :arrow_down:

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 64f8cfe...a6a3148. Read the comment docs.

codecov[bot] avatar Aug 05 '22 07:08 codecov[bot]

Overall, this looks great, thank you.

samuelcolvin avatar Aug 05 '22 08:08 samuelcolvin

Feedback addressed. Still have to:

  • Write some more tests for edge cases and ack/delete features
  • Do a smoke test against a real SQS queue to make sure I'm making the API calls correctly

adriangb avatar Aug 06 '22 11:08 adriangb

all done. smoke tests helped me fix the API calls. I'm not including them because they're super crude, but given the stability of the HTTP API I think we should be fine just ensuring we don't break what's there right now. What do you think @samuelcolvin, is this an acceptable testing strategy?

adriangb avatar Aug 06 '22 17:08 adriangb

Yes I think so the aws4 Auth is tested against aws on commits to master.

I'll look in detail on Monday.

samuelcolvin avatar Aug 06 '22 17:08 samuelcolvin

Thanks so much for this. Sorry it took so long.

samuelcolvin avatar Jan 10 '23 16:01 samuelcolvin

No worries, I'm very glad this is in now!

adriangb avatar Jan 10 '23 16:01 adriangb

I'll do a new release soon. Just checking this works properly with CloudFlare's R2.

samuelcolvin avatar Jan 10 '23 16:01 samuelcolvin