Improve the processing of faucet_host URL input (Draft PR) #676
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- first draft of integration tests for the new URL processing function
…rbose error handling
from urllib.parse import urlparse, urlunparse
def process_faucet_host_url(input_url: str) -> str:
"""
Construct a URL from the given input string.
Args:
- input_url (str): The input string that may or may not include a protocol,
and may or may not have a path.
Returns:
- str: The constructed URL with https as the default protocol and /accounts as the default path.
"""
# Parse the input URL to identify its components.
parsed_url = urlparse(input_url)
# If the input string includes a protocol (e.g., "https://"), urlparse will correctly parse it.
# Scheme refers to the protocol (e.g., "https", "http").
scheme = parsed_url.scheme if parsed_url.scheme else 'https'
# Netloc is the network location part, which usually includes the domain name.
# For input "https://abcd.com", netloc is "abcd.com".
# If no protocol is provided, the domain might be parsed as the path.
# Consider the input string "abcd.com". If you were to parse this string using urlparse
# without manually prepending a protocol (like http:// or https://), the parsing logic would
# interpret "abcd.com" not as the network location part (or domain) of the URL, but rather
# as the path component. This is because urlparse expects a scheme (protocol) to correctly
# identify the parts of the URL.
# Hence, we check if netloc is present; if not, assume the path is actually the netloc.
netloc = parsed_url.netloc if parsed_url.netloc else parsed_url.path
path = parsed_url.path if parsed_url.netloc else ''
# If no specific path is provided, append '/accounts' to the URL.
# For input "abcd.com", the constructed path will be "/accounts".
if not path:
path = '/accounts'
# Construct the final URL by reassembling its components.
final_url = urlunparse((scheme, netloc, path, '', '', ''))
return final_urlTestsfrom unittest import TestCase
from utils import process_faucet_host_url
class TestProcessFaucetHostURL(TestCase):
"""Test process_faucet_host_url."""
def test_process_faucet_host_url_no_protocol(self) -> None:
"""Test with domain only, no protocol. Lacks a protocol and path, so it defaults to https://abcd.com/accounts"""
input_url = "faucet.devnet.rippletest.net"
expected_url = "https://faucet.devnet.rippletest.net/accounts"
self.assertEqual(process_faucet_host_url(input_url), expected_url)
def test_process_faucet_host_url_with_https(self) -> None:
"""Test with HTTPS protocol specified. Has a complete URL but lacks a path, thus "/accounts" is appended."""
input_url = "https://faucet.devnet.rippletest.net"
expected_url = "https://faucet.devnet.rippletest.net/accounts"
self.assertEqual(process_faucet_host_url(input_url), expected_url)
def test_process_faucet_host_url_with_path(self) -> None:
"""Test with domain and path, no protocol. Lacks a protocol, defaults to "https://", and already specifies a path."""
input_url = "faucet.devnet.rippletest.net/accounts"
expected_url = "https://faucet.devnet.rippletest.net/accounts"
self.assertEqual(process_faucet_host_url(input_url), expected_url)
def test_process_faucet_host_url_full(self) -> None:
"""Test with full URL."""
input_url = "https://faucet.devnet.rippletest.net/accounts"
expected_url = "https://faucet.devnet.rippletest.net/accounts"
self.assertEqual(process_faucet_host_url(input_url), expected_url)
def test_process_faucet_host_url_different_protocol(self) -> None:
"""Test with a non-HTTP protocol specified."""
input_url = "ftp://faucet.devnet.rippletest.net"
expected_url = "ftp://faucet.devnet.rippletest.net/accounts"
self.assertEqual(process_faucet_host_url(input_url), expected_url) |
|
Hey @justinr1234 , The test cases are failing when the Here is the stacktrace: This is a known behavior of the |
|
@ckeshava we should be able to handle stripping trailing slashes I’d say |
9 tasks
|
This issue is being solved by #690, hence closing this PR |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
High Level Overview of Change
This PR seeks to address #364
Context of Change
This is a slight modification in the handling of the faucet_host URL. The developer can have the flexibility to specify the resource paths (like
/accounts) and internet protocols ('https://') in the input.Type of Change
Did you update CHANGELOG.md?
Test Plan
How can I test the use of custom faucet-host? I used
faucet-nft.ripple.com, but I'm gettingConnection Timeouterrors. The existing integration tests use the Testnet, Devnet and hooks faucet to validate the code, but not any custom faucets.