add server sync method script#1036
Conversation
58eda95 to
c127987
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces a suite of utility scripts and documentation for interacting with the LightX2V server's image generation APIs. The additions include benchmarking tools for S3 latency, scripts for asynchronous and synchronous Text-to-Image (T2I) and Image-to-Image (I2I) tasks using base64 or presigned URLs, and a comprehensive usage guide. The review feedback identifies two technical improvements: replacing time.time() with time.monotonic() for more robust deadline calculations in polling loops, and fixing a logic error where a PUT presigned URL could incorrectly be used for download verification, leading to a 403 error.
| deadline = time.time() + timeout_seconds | ||
|
|
||
| while time.time() < deadline: |
There was a problem hiding this comment.
Using time.time() for calculating deadlines and measuring durations is discouraged because it is not monotonic. System clock adjustments (e.g., via NTP) can cause the loop to terminate prematurely or hang. It is better to use time.monotonic() for this purpose.
| deadline = time.time() + timeout_seconds | |
| while time.time() < deadline: | |
| deadline = time.monotonic() + timeout_seconds | |
| while time.monotonic() < deadline: |
| output_path=args.download_output, | ||
| timeout_seconds=args.download_timeout_seconds, | ||
| ) | ||
| print(f"Downloaded uploaded image to: {downloaded_image}") |
There was a problem hiding this comment.
The fallback to result.get("presigned_url") for download verification is problematic. In presigned_url mode, the server returns the same PUT URL it was provided. Attempting a GET request on a PUT presigned URL will result in a 403 Forbidden error from S3. If the user provides a custom --presigned_url, they should also provide a corresponding --download_url for verification, or the script should skip the download check if no GET URL is available.
| print(f"Downloaded uploaded image to: {downloaded_image}") | |
| download_url = args.download_url or resolved_download_url |
No description provided.