Conversation
lambda_src/drivers/process_https.py
Outdated
lambda_src/drivers/process_https.py
Outdated
There was a problem hiding this comment.
This doesn't seem to print anything important it seems -- the req object has no special __repr__ or __str__, and it only ends up printing the class and the memory address of the object.
Something like <urllib.request.Request object at 0x7fe94f185310>. This could be removed maybe?
lambda_src/drivers/process_https.py
Outdated
There was a problem hiding this comment.
Hash for comparing two requests
lambda_src/lambda_function.py
Outdated
There was a problem hiding this comment.
Made this change so that this line actually gets the bytes, instead of the string.
| { | ||
| 'body': response_body, | ||
| 'headers': response_headers, | ||
| 'status': response_status, |
There was a problem hiding this comment.
Added status code returned for the request too.
lambda_src/lambda_function.py
Outdated
There was a problem hiding this comment.
Calculating the driver invocation time.
lambda_src/drivers/destination_s3.py
Outdated
There was a problem hiding this comment.
Replace bucket with uri here.
Writing result of row "%s" of batch "%s" to URI "%s".
lambda_src/drivers/process_https.py
Outdated
There was a problem hiding this comment.
Make this similar to L95
lambda_src/drivers/process_https.py
Outdated
There was a problem hiding this comment.
Body instead of headers
lambda_src/drivers/process_https.py
Outdated
There was a problem hiding this comment.
Move up to the req, auth host comparison, log both of them
lambda_src/drivers/process_https.py
Outdated
There was a problem hiding this comment.
md5 and call it an etag
| LOG.info( | ||
| 'Got the response with status code: %d and body size: %d bytes, for URL: %s.', | ||
| response_status, | ||
| len(res_body), |
There was a problem hiding this comment.
use res.body/similar instead
There was a problem hiding this comment.
It doesn't seem like the result object has an attribute like that
There was a problem hiding this comment.
we should also include how long the request took into a separate info log
we should also respond with the time the request / response took into the metadata returned when the user turns verbose option on
lambda_src/lambda_function.py
Outdated
lambda_src/lambda_function.py
Outdated
lambda_src/lambda_function.py
Outdated
There was a problem hiding this comment.
similar as above, call tree can be debug
lambda_src/lambda_function.py
Outdated
lambda_src/lambda_function.py
Outdated
There was a problem hiding this comment.
maybe skip, since specifc to htttps
lambda_src/lambda_function.py
Outdated
There was a problem hiding this comment.
Let the S3 logging be inside the driver.
Driver agnostic, say storage.
There was a problem hiding this comment.
Just the time and the size of the metadata to be returned, since it's not necessarily a write.
| response_length, | ||
| ) | ||
| else: | ||
| data_dumps = dumps({'data': res_data}, default=str) |
There was a problem hiding this comment.
log pre gzip size too, and use len of bite array instead of string
lambda_src/lambda_function.py
Outdated
There was a problem hiding this comment.
implicit, delete
| elif method == 'POST': | ||
| LOG.info('Invocation: synchronous.') | ||
| return sync_flow(event, context) | ||
| elif method == 'GET': |
There was a problem hiding this comment.
Add log for polling, too
lambda_src/drivers/destination_s3.py
Outdated
There was a problem hiding this comment.
should be .error
lambda_src/drivers/destination_s3.py
Outdated
2 participants
This PR—
Related TF resources: Snowflake-Labs/terraform-snowflake-api-integration-with-geff-aws#78