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

Doc rework #1698

Merged
merged 22 commits into from Nov 14, 2019
Merged

Doc rework #1698

merged 22 commits into from Nov 14, 2019

Conversation

Lagicrus
Copy link
Contributor

In reference to #1691 I have moved all markdown docs to rst.
The discussion over why we went for rst vs md can be seen in full in the issue chat.

There is a minor bug with the table in the configuration page where the top row doesn't seem to have splits, but I am not sure if that is a rst "feature" vs an actual bug.
Also, the routing on the left nav bar for some reason seems to be broken and I am unsure as to why.

I ran a tox check but it skipped due to win32 not in list
There are a large number of errors when I build it with sphinx, but those are to do with api_reference and _sanic which are files I have not touched, nor understand.

@Lagicrus Lagicrus mentioned this pull request Oct 11, 2019
@codecov
Copy link

codecov bot commented Oct 11, 2019

Codecov Report

Merging #1698 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1698   +/-   ##
=======================================
  Coverage   92.11%   92.11%           
=======================================
  Files          22       22           
  Lines        2218     2218           
  Branches      411      411           
=======================================
  Hits         2043     2043           
  Misses        137      137           
  Partials       38       38

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e81a8ce...c3c80f1. Read the comment docs.

@sjsadowski
Copy link
Contributor

Looks good. Once you move it out of draft status, I'll approve it. Thanks for doing this!

@harshanarayana
Copy link
Contributor

@Lagicrus this is great work. Let me do a quick clone of your branch and check if the docs render. I'm sure you have done this already.

@Lagicrus
Copy link
Contributor Author

Looks good. Once you move it out of draft status, I'll approve it. Thanks for doing this!

Thanks :)

@Lagicrus
Copy link
Contributor Author

@Lagicrus this is great work. Let me do a quick clone of your branch and check if the docs render. I'm sure you have done this already.

Awesome
Yeah, I tested it after every doc change & I was monitoring errors via my IDE for how stuff broke,
which is where i learnt where you have the .. code-block: python you have to have the code indented and a gap as well

If you can check the table bug which I mentioned which is on the config page and see what you think, it renders but for some reason the headers are one block
And the side nav is broke for routing. Like if you click routing on the sidenav instead of opening the tiered bit for it, it puts it instead into the main indent

To show what I mean
this is request data
image which as you can see works perfectly fine
But when I click on routing I get this
image which between routing and the highlighted block at the bottom of it should be indented. Unsure how this broke

@Lagicrus
Copy link
Contributor Author

@harshanarayana how is it looking on your end?

@harshanarayana
Copy link
Contributor

@Lagicrus I am having the same issue on my side. Trying to find out what is causing it. Give me till the End of today and let me see if I can narrow down the issue to something specific.

@Lagicrus
Copy link
Contributor Author

@Lagicrus I am having the same issue on my side. Trying to find out what is causing it. Give me till the End of today and let me see if I can narrow down the issue to something specific.

@harshanarayana any luck so far?

@sjsadowski
Copy link
Contributor

@harshanarayana any chance you can circle back on this in the next few days?

@harshanarayana
Copy link
Contributor

@sjsadowski Sorry for the delay. I am checking this as I type this message.

@harshanarayana
Copy link
Contributor

From aa65f5bed4d61ccbf362f043fa1ad8462763f3b5 Mon Sep 17 00:00:00 2001
From: Harsha Narayana <harsha2k4@gmail.com>
Date: Sat, 2 Nov 2019 22:01:01 +0530
Subject: [PATCH] doc: fix rst templating and few other warnings

Signed-off-by: Harsha Narayana <harsha2k4@gmail.com>
---
 CONTRIBUTING.rst            |  2 +-
 docs/sanic/middleware.rst   |  4 ++--
 docs/sanic/request_data.rst | 29 ++++++++++++++---------------
 docs/sanic/routing.rst      | 16 ++++++++--------
 docs/sanic/testing.rst      |  2 +-
 sanic/request.py            |  5 +++++
 6 files changed, 31 insertions(+), 27 deletions(-)

diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index 5d969477..b08a60db 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -68,7 +68,7 @@ run all unittests, perform lint and other checks.
 Run unittests
 -------------
 
-``tox`` environment -> ``[testenv]`
+``tox`` environment -> ``[testenv]``
 
 To execute only unittests, run ``tox`` with environment like so:
 
diff --git a/docs/sanic/middleware.rst b/docs/sanic/middleware.rst
index e39340ad..3ab27dab 100644
--- a/docs/sanic/middleware.rst
+++ b/docs/sanic/middleware.rst
@@ -68,8 +68,7 @@ The three middlewares are executed in order:
 
 1. The first request middleware **add_key** adds a new key `foo` into request context.
 2. Request is routed to handler **index**, which gets the key from context and returns a text response.
-3. The first response middleware **custom_banner** changes the HTTP response header *Server* to
-say *Fake-Server*
+3. The first response middleware **custom_banner** changes the HTTP response header *Server* to say *Fake-Server*
 4. The second response middleware **prevent_xss** adds the HTTP header for preventing Cross-Site-Scripting (XSS) attacks.
 
 Responding early
@@ -116,6 +115,7 @@ These listeners are implemented as decorators on functions which accept the app
 For example:
 
 .. code-block:: python
+
     @app.listener('before_server_start')
     async def setup_db(app, loop):
         app.db = await db_setup()
diff --git a/docs/sanic/request_data.rst b/docs/sanic/request_data.rst
index 8866cf53..ffc182ce 100644
--- a/docs/sanic/request_data.rst
+++ b/docs/sanic/request_data.rst
@@ -18,11 +18,11 @@ The following variables are accessible as properties on `Request` objects:
 
 
 - `args` (dict) - Query string variables. A query string is the section of a
-  URL that resembles `?key1=value1&key2=value2`. If that URL were to be parsed,
-  the `args` dictionary would look like `{'key1': ['value1'], 'key2': ['value2']}`.
-  The request's `query_string` variable holds the unparsed string value.
-  Property is providing the default parsing strategy. If you would like to change it look to the section below
-  (`Changing the default parsing rules of the queryset`).
+  URL that resembles ``?key1=value1&key2=value2``
+
+If that URL were to be parsed, the `args` dictionary would look like ``{'key1': ['value1'], 'key2': ['value2']}``.
+The request's `query_string` variable holds the unparsed string value. Property is providing the default parsing
+strategy. If you would like to change it look to the section below (`Changing the default parsing rules of the queryset`).
 
 .. code-block:: python
 
@@ -34,15 +34,14 @@ The following variables are accessible as properties on `Request` objects:
 
 - `query_args` (list) - On many cases you would need to access the url arguments in
   a less packed form. `query_args` is the list of `(key, value)` tuples.
-  Property is providing the default parsing strategy. If you would like to change it look to the section below
-  (`Changing the default parsing rules of the queryset`).
-  For the same previous URL queryset `?key1=value1&key2=value2`, the
-  `query_args` list would look like `[('key1', 'value1'), ('key2', 'value2')]`.
-  And in case of the multiple params with the same key like `?key1=value1&key2=value2&key1=value3`
-  the `query_args` list would look like `[('key1', 'value1'), ('key2', 'value2'), ('key1', 'value3')]`.
 
-  The difference between Request.args and Request.query_args
-  for the queryset `?key1=value1&key2=value2&key1=value3`
+Property is providing the default parsing strategy. If you would like to change it look to the section below
+(`Changing the default parsing rules of the queryset`). For the same previous URL queryset
+``?key1=value1&key2=value2``, the `query_args` list would look like ``[('key1', 'value1'), ('key2', 'value2')]``.
+And in case of the multiple params with the same key like ``?key1=value1&key2=value2&key1=value3``
+the `query_args` list would look like ``[('key1', 'value1'), ('key2', 'value2'), ('key1', 'value3')]``.
+
+The difference between Request.args and Request.query_args for the queryset ``?key1=value1&key2=value2&key1=value3``
 
 .. code-block:: python
 
@@ -68,7 +67,7 @@ The following variables are accessible as properties on `Request` objects:
 
   Output
 
-.. code-block:: JSON
+.. code-block:: json
 
     {
         "parsed":true,
@@ -79,7 +78,7 @@ The following variables are accessible as properties on `Request` objects:
         "query_args":[["key1","value1"],["key2","value2"],["key1","value3"]]
     }
 
-  `raw_args` contains only the first entry of `key1`. Will be deprecated in the future versions.
+- `raw_args` contains only the first entry of `key1`. Will be deprecated in the future versions.
 
 - `files` (dictionary of `File` objects) - List of files that have a name, body, and type
 
diff --git a/docs/sanic/routing.rst b/docs/sanic/routing.rst
index 2fdca59e..b073ed50 100644
--- a/docs/sanic/routing.rst
+++ b/docs/sanic/routing.rst
@@ -22,7 +22,7 @@ Sanic handler functions must be defined using the `async def` syntax, as they
 are asynchronous functions.
 
 Request parameters
-------------------
+==================
 
 Sanic comes with a basic router that supports request parameters.
 
@@ -115,7 +115,7 @@ If no type is set then a string is expected. The argument given to the function
     `str` is not a valid type tag. If you want `str` recognition then you must use `string`
 
 HTTP request types
-------------------
+==================
 
 By default, a route defined on a URL will be available for only GET requests to that URL.
 However, the `@app.route` decorator accepts an optional parameter, `methods`,
@@ -161,7 +161,7 @@ There are also shorthand method decorators:
         return text('GET request - {}'.format(request.args))
 
 The `add_route` method
-----------------------
+======================
 
 As we have seen, routes are often specified using the `@app.route` decorator.
 However, this decorator is really just a wrapper for the `app.add_route`
@@ -187,7 +187,7 @@ method, which is used as follows:
     app.add_route(person_handler2, '/person/<name:[A-z]>', methods=['GET'])
 
 URL building with `url_for`
----------------------------
+===========================
 
 Sanic provides a `url_for` method, to generate URLs based on the handler method name. This is useful if you want to avoid hardcoding url paths into your app; instead, you can just reference the handler name. For example:
 
@@ -244,7 +244,7 @@ Other things to keep in mind when using `url_for`:
 - All valid parameters must be passed to `url_for` to build a URL. If a parameter is not supplied, or if a parameter does not match the specified type, a `URLBuildError` will be raised.
 
 WebSocket routes
-----------------
+================
 
 Routes for the WebSocket protocol can be defined with the `@app.websocket`
 decorator:
@@ -279,7 +279,7 @@ package by Aymeric Augustin.
 
 
 About `strict_slashes`
-----------------------
+======================
 
 You can make `routes` strict to trailing slash or not, it's configurable.
 
@@ -340,7 +340,7 @@ found in the above order will be applied to the route in question.
         return text("strict_slashes applicable from blueprint level")
 
 User defined route name
------------------------
+=======================
 
 A custom route name can be used by passing a `name` argument while registering the route which will
 override the default route name generated using the `handler.__name__` attribute.
@@ -403,7 +403,7 @@ override the default route name generated using the `handler.__name__` attribute
     # app.url_for('post_handler') == '/post'
 
 Build URL for static files
---------------------------
+==========================
 
 Sanic supports using `url_for` method to build static file urls. In case if the static url
 is pointing to a directory, `filename` parameter to the `url_for` can be ignored.   q
diff --git a/docs/sanic/testing.rst b/docs/sanic/testing.rst
index 22170907..5dba2ab4 100644
--- a/docs/sanic/testing.rst
+++ b/docs/sanic/testing.rst
@@ -31,7 +31,7 @@ The `test_client` methods accept the following arguments and keyword arguments:
   original request will be returned by the function. If set to `True`, the
   return value is a tuple of `(request, response)`, if `False` only the
   response is returned.
-- `server_kwargs` *(default `{}`) a dict of additional arguments to pass into `app.run` before the test request is run.
+- `server_kwargs` *(default `{}`)* a dict of additional arguments to pass into `app.run` before the test request is run.
 - `debug` *(default `False`)* A boolean which determines whether to run the server in debug mode.
 
 The function further takes the `*request_args` and `**request_kwargs`, which are passed directly to the request.
diff --git a/sanic/request.py b/sanic/request.py
index 972f7dbf..d95f9156 100644
--- a/sanic/request.py
+++ b/sanic/request.py
@@ -125,26 +125,31 @@ def __repr__(self):
 
     def get(self, key, default=None):
         """.. deprecated:: 19.9
+
            Custom context is now stored in `request.custom_context.yourkey`"""
         return self.ctx.__dict__.get(key, default)
 
     def __contains__(self, key):
         """.. deprecated:: 19.9
+
            Custom context is now stored in `request.custom_context.yourkey`"""
         return key in self.ctx.__dict__
 
     def __getitem__(self, key):
         """.. deprecated:: 19.9
+
            Custom context is now stored in `request.custom_context.yourkey`"""
         return self.ctx.__dict__[key]
 
     def __delitem__(self, key):
         """.. deprecated:: 19.9
+
            Custom context is now stored in `request.custom_context.yourkey`"""
         del self.ctx.__dict__[key]
 
     def __setitem__(self, key, value):
         """.. deprecated:: 19.9
+
            Custom context is now stored in `request.custom_context.yourkey`"""
         setattr(self.ctx, key, value)

@Lagicrus Here you go. Apply this diff and that should fix your problem and a few other warnings too.

Tried to push to your branch directly but doesn't seem to work

sanic on  doc-rework [$] via 🅒 sanic via 🐍 3.6.6 at ☸️  minikube
➜ git push lag doc-rework
ERROR: Permission to Lagicrus/sanic.git denied to harshanarayana.
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

@harshanarayana
Copy link
Contributor

@Lagicrus
Copy link
Contributor Author

Not sure why it didn't work for your push 🤔
But will try

@Lagicrus
Copy link
Contributor Author

@harshanarayana currently having issues applying your patch. I download it then do git apply --stat aa65f5bed4d61ccbf362f043fa1ad8462763f3b5.patch and it complains about error: corrupt patch at line 23 do you want to fix it? Or would it be easier for me to fix whatever perm issues you are having which is confusing me?

@harshanarayana
Copy link
Contributor

@Lagicrus Ah! I think I know the problem. The commit hash on mine might be different from your since I pulled your branch and rebased it agianst the upstream first.

That said, you should be able to detect the changes from the patch file directly and apply them to your code as applicable. Basically replace the line with - in your file with the contents as in + line that's all that needs to be done. Let me know if you need anything else from me

Co-Authored-By: Harsha Narayana <harsha2k4@gmail.com>
@Lagicrus
Copy link
Contributor Author

Ok, no idea why appveyor broke so that's fun

@harshanarayana @sjsadowski , I think it is ready now so feel free to sanity check this and I will make it ready for review 👍

There are a lot of warnings but they haven't changed since I first pulled the project, no idea how to fix the 900 ones I get :D
In order
https://starb.in/FBnTOB.sql
https://starb.in/IjfQAH.sql
https://starb.in/ohLRCF.sql

Not sure if a me issue or not

@harshanarayana
Copy link
Contributor

https://starb.in/FBnTOB.sql
https://starb.in/IjfQAH.sql
https://starb.in/ohLRCF.sql

@Lagicrus Mind sharing the details of how you managed to extract these details? Last time I checked, these warnings weren't where while building the documentation.

make docs shows them?

@harshanarayana
Copy link
Contributor

@yunstanford @ahopkins @sjsadowski can we get a retrigger of travis job as well as the appveyor job? Looks like the travis job was never triggered?

@sjsadowski
Copy link
Contributor

Let me see if I can get it to re-run...

@sjsadowski
Copy link
Contributor

So the travis build is stuck here: fd48847 and passing, however it is not pulling the newer commits despite expiring the cache. I'm trying to figure out wth is going on.

@Lagicrus
Copy link
Contributor Author

https://starb.in/FBnTOB.sql
https://starb.in/IjfQAH.sql
https://starb.in/ohLRCF.sql

@Lagicrus Mind sharing the details of how you managed to extract these details? Last time I checked, these warnings weren't where while building the documentation.

make docs shows them?

Manually doing
sphinx-apidoc -fo docs/_api/ sanic
sphinx-build -b html docs docs/_build
(Windows 10)

from https://sanic.readthedocs.io/en/latest/sanic/contributing.html#documentation
Sounds like it could just be a me issue and i need to install more python or sphinx stuff then?

@harshanarayana
Copy link
Contributor

So the travis build is stuck here: fd48847 and passing, however it is not pulling the newer commits despite expiring the cache. I'm trying to figure out wth is going on.

Thanks @sjsadowski

@sjsadowski sjsadowski marked this pull request as ready for review November 14, 2019 15:36
@sjsadowski
Copy link
Contributor

Turns out this is a known (and still open) issue: https://travis-ci.community/t/draft-pull-requests-not-being-built/2434

Since we're looking to do final validation and then merge this, I've taken it out of draft status and will try to build again

@sjsadowski
Copy link
Contributor

@Lagicrus can you fix the conflict? I think we should be able to build then...

@Lagicrus
Copy link
Contributor Author

Done I think @sjsadowski , now we wait for those checks

@sjsadowski
Copy link
Contributor

@Lagicrus success!

@sjsadowski sjsadowski merged commit a4185a0 into sanic-org:master Nov 14, 2019
@Lagicrus
Copy link
Contributor Author

Woop! Thanks for the help with getting this moved through @sjsadowski @harshanarayana
Any idea when we can see this live?

@Lagicrus Lagicrus deleted the doc-rework branch November 14, 2019 20:12
@sjsadowski
Copy link
Contributor

@Lagicrus thanks for your work on it. I think we need to cut a release for it to go up... I'll double check.

@Lagicrus
Copy link
Contributor Author

Hm, Closes #1691

@harshanarayana
Copy link
Contributor

@Lagicrus Awesome :) They are already live. If you browse the https://sanic.readthedocs.io this is from the head of the master branch. So your changes are already there 🥂

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

3 participants