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

Removed the "api_endpoint" setting from everywhere (in this repo) #821

Merged
merged 3 commits into from Dec 6, 2016

Conversation

ttmc
Copy link
Contributor

@ttmc ttmc commented Nov 15, 2016

As described in issue #780, the api_endpoint setting isn't actually used anywhere (as far as I can tell), so I removed it from everywhere.

I added some docs to the page about the HTTP API to explain how the API Root URL is actually determined.

  • Ran all unit tests. They passed.
  • Ran flake8. It passed.
  • Tested an AWS deployment. It worked.
  • Played around with Docker based on edited Dockerfile. It seemed to work (but the bigchaindb load command seems to be broken now; that's another issue)

Resolves #780

@ttmc
Copy link
Contributor Author

ttmc commented Nov 15, 2016

@sbellem Could you try out the Dockerfile and Docker Compose file to see if that stuff is still okay? I did some testing but I'm not a Docker pro.

I created a related issue to update the BigchainDB Python Driver (docs etc.): bigchaindb/bigchaindb-driver#142

@sohkai
Copy link
Contributor

sohkai commented Nov 16, 2016

Oh interesting, we have BIGCHAINDB_SERVER_BIND that actually sets the server host and port. There's a lot of dockerfiles to go through and check in all the different repos, but since this env variable actually isn't used, nothing shouldn't be affected... right?

@ttmc
Copy link
Contributor Author

ttmc commented Nov 16, 2016

BIGCHAINDB_SERVER_BIND isn't the whole story of how the public HTTP API URLs are determined. I added some documentation about how those are actually determined (in this PR, in docs/server/source/drivers-clients/http-client-server-api.rst).

@sohkai
Copy link
Contributor

sohkai commented Nov 16, 2016

Just grepped through my local repos (which should include most of our docker-related files) and only found two instances of BIGCHAINDB_API_ENDPOINT (both depend on the python driver). Once we confirm that the driver doesn't need it, I'll remove those instances as well.

Copy link
Contributor

@vrde vrde left a comment

Choose a reason for hiding this comment

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

I did some manual testing and I was able to run configure, start, and show-config.

@codecov-io
Copy link

Current coverage is 96.64% (diff: 100%)

Merging #821 into master will not change coverage

@@             master       #821   diff @@
==========================================
  Files            27         27          
  Lines          1669       1669          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           1613       1613          
  Misses           56         56          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 97ff0ef...adde849

@ttmc
Copy link
Contributor Author

ttmc commented Dec 2, 2016

This pull request can probably be merged around the same time as bigchaindb/bigchaindb-driver#171

@ttmc
Copy link
Contributor Author

ttmc commented Dec 6, 2016

bigchaindb/bigchaindb-driver#171 was merged earlier today so I'm merging this pull request now. Farewell, "api_endpoint"

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

Successfully merging this pull request may close these issues.

None yet

5 participants