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

Default values of Client/connect() are not documented #150

Closed
AuHau opened this issue Jan 25, 2019 · 3 comments
Closed

Default values of Client/connect() are not documented #150

AuHau opened this issue Jan 25, 2019 · 3 comments
Assignees

Comments

@AuHau
Copy link
Member

AuHau commented Jan 25, 2019

The #7 was reintroduced I guess by some refactoring. Passing None to Client/connect() will result in no default values used and a not much informative exception:

File "/Users/c5272397/dev/ipfs-publish/.direnv/python-3.6.5/lib/python3.6/site-packages/ipfsapi/http.py", line 143, in __init__
    if not re.match('^https?://', host.lower()):
AttributeError: 'NoneType' object has no attribute 'lower'

I am happy to provide refix, I am only not sure how to proceed because of the on-going renaming (#148). Should I wait until merged, target some specific branch or something else? Please let me know.

@AuHau AuHau changed the title Client accept None as host None passed to host into Client/connect() won't result in defaults used Jan 25, 2019
@ntninja
Copy link
Contributor

ntninja commented Jan 27, 2019

Thanks for your feedback! I've finally received some feedback on the discussion regarding the files-APIs in #146, so there will finally be progress now on the py-ipfs-api/py-ipfs-http-client rename+redesign thing. As such I'd ask you to wait another week or so before proposing any patches.

Two comments on your post however:

  1. I'm really convinced we're talking about a bug here, since the default values for the host and port are ipfsapi.DEFAULT_HOST & ipfsapi.DEFAULT_PORT respectively. I do see however that the documentation doesn't show this fact unfortunately – which I definitely consider a bug on the other hand.
  2. Part of the aforementioned redesign will be the fact the client will use a multiaddr instead of the host+port+base triplet currently in use, hence why I'm asking you to delay any patches for now.

@AuHau
Copy link
Member Author

AuHau commented Jan 27, 2019

Okey! I will wait and follow the #149 and after completion then revisit.

Thanks for the reply!

AuHau added a commit to AuHau/py-ipfs-api that referenced this issue Jan 31, 2019
AuHau added a commit to AuHau/py-ipfs-api that referenced this issue Feb 9, 2019
@ntninja
Copy link
Contributor

ntninja commented Feb 15, 2019

As mentioned in #150 this won't be “fixed” by allowing None; rather we'll update the documentation to highlight that ipfshttpclient.DEFAULT_{HOST,PORT,BASE} are the respective default values.

@ntninja ntninja added this to the ipfs-http-client v0.4.4 milestone Feb 15, 2019
@ntninja ntninja changed the title None passed to host into Client/connect() won't result in defaults used Default values of Client/connect() are not documented Feb 15, 2019
ntninja added a commit to ntninja/py-ipfs-api-client that referenced this issue Feb 16, 2019
  * Better signature for the `connect` function (fixes ipfs-shipyard#150)
  * Document the `stream` parameter
  * Return values are now *documented*, not *commented* on (syntactial change)
  * Document the JSON object keys of several methods separately
@ghost ghost assigned ntninja Feb 16, 2019
@ghost ghost added the in progress label Feb 16, 2019
ntninja added a commit to ntninja/py-ipfs-api-client that referenced this issue Mar 5, 2019
  * Better signature for the `connect` function (fixes ipfs-shipyard#150)
  * Document the `stream` parameter
  * Return values are now *documented*, not *commented* on (syntactial change)
  * Document the JSON object keys of several methods separately
ntninja added a commit that referenced this issue Mar 5, 2019
  * Better signature for the `connect` function (fixes #150)
  * Document the `stream` parameter
  * Return values are now *documented*, not *commented* on (syntactial change)
  * Document the JSON object keys of several methods separately
@ntninja ntninja closed this as completed May 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants