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

Adding the timeout flag for fileserver command and setting default timeout to 60 seconds #247

Merged

Conversation

KaminFay
Copy link
Contributor

Please check below, if the PR fulfills these requirements:

  • Commit(s) and code follow the repositories guidelines.
  • Test(s) have been added or updated to support these change(s).
  • Doc(s) have been added or updated to support these change(s).

Associated Links:

Types of Changes:

  • Bugfix

Proposed Changes:

  • Changing the default fileserver http timeout from 15 seconds to 60 seconds to account for larger files.
  • Adding an additional flag to the fileserver command to manipulate the desired timeout. This will allow end users to stipulate the timeout if they require excessively large file transfers or have a slow network connection.
  • Proposed new command:
Serve the file server

Usage:
  hauler store serve fileserver [flags]

Flags:
      --directory string   Directory to use for backend.  Defaults to $PWD/fileserver (default "fileserver")
  -h, --help               help for fileserver
  -p, --port int           Port to listen on. (default 8080)
  -t, --timeout int        Set the http request timeout duration in seconds for both reads and write. (default 60)

Global Flags:
      --cache string       (deprecated flag and currently not used)
  -l, --log-level string    (default "info")
  -s, --store string       Location to create store at (default "store")

Verification/Testing of Changes:

  1. Load a large file into the store.
  2. Serve the fileserver with the following flag --timeout 1
  3. Attempt to curl for the large file and note the immediate timeout after 1 second.
  4. Close the fileserver
  5. Reserve with either no flag (60 second timeout) or a large timeout --timeout 300
  6. Attempt to curl for the large file and and note the download proceeds with no issues.

Additional Context:

  • Edit: Opened a new PR with signed commits.
  • This was a rather simple change that meets both requested ideas from the linked github issue.
  1. Extends the default timeout as 15 seconds appears to be too short in some cases.
  2. Allows the end user to support their needs with a variable timeout.

@KaminFay KaminFay marked this pull request as ready for review May 28, 2024 19:29
@zackbradys zackbradys added bug Something isn't working size/S Denotes an issue/PR requiring a relatively small amount of work labels May 28, 2024
@zackbradys zackbradys self-assigned this May 28, 2024
Copy link
Member

@zackbradys zackbradys left a comment

Choose a reason for hiding this comment

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

Thank you for your descriptive PR, code contribution, and relevant updates to the hauler-docs. LGTM!! 🚀 🚀

@zackbradys zackbradys merged commit ebab7f3 into hauler-dev:main Jun 4, 2024
1 check passed
@zackbradys zackbradys added this to the Hauler v1.0.5 milestone Aug 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size/S Denotes an issue/PR requiring a relatively small amount of work
Projects
Status: Resolved
Development

Successfully merging this pull request may close these issues.

2 participants