API Info v2 #157

kyamiko · 2017-05-21T20:54:20Z

Added API Info query.

As close to API Upload v2 behavior as possible.

TheAMM · 2017-05-22T21:45:09Z

nyaa/api_handler.py

+    if torrent.user and (viewer == torrent.user or viewer.is_moderator):
+        submitter = torrent.user.username
+
+    files = ''


Just default to a {} - mixing types is not nice for API users.

TheAMM · 2017-05-22T21:46:52Z

nyaa/api_handler.py

+    matchHASH = re.match(INFO_HASH_PATTERN, torrent_id_or_hash)
+
+    if matchID:
+        torrent = models.Torrent.by_id(torrent_id_or_hash)


torrent_id_or_hash is a str here - Torrent.id is an int.

TheAMM · 2017-05-22T21:47:44Z

migrations/versions/3001f79b7722_add_torrents.uploader_ip.py

@@ -1,7 +1,7 @@
 """Add uploader_ip column to torrents table.

 Revision ID: 3001f79b7722
-Revises: 
+Revises:


Why is this touched?

@kyamiko You should revert that change, IMO

TheAMM · 2017-05-22T21:48:50Z

nyaa/api_handler.py

+        'url': flask.url_for('view_torrent', torrent_id=torrent.id, _external=True),
+        'id': torrent.id,
+        'name': torrent.display_name,
+        'hash': torrent.info_hash.hex(),


See https://github.com/nyaadevs/nyaa/blob/master/nyaa/models.py#L136

TheAMM · 2017-05-22T21:50:14Z

nyaa/api_handler.py

+INFO_HASH_PATTERN = '^[0-9a-fA-F]{40}$'  # INFO_HASH as string
+
+
+@api_blueprint.route('/v2/info/<torrent_id_or_hash>', methods=['GET'])


This doesn't have to be v2, does it? We never really had a proper v1 api set.

No it doesn't. I tried to keep it as close to /v2/upload as possible.

TheAMM · 2017-05-22T21:54:47Z

nyaa/api_handler.py

+    if torrent.deleted and not (viewer and viewer.is_superadmin):
+        return flask.jsonify({'errors': ['Query was not a valid id or hash.']}), 400
+
+    submitter = 'anonymous'


Since this is a name, I'd make it Anonymous - or possibly, since /user/Anonymous can exist, None

TheAMM · 2017-05-22T21:55:55Z

nyaa/api_handler.py

+        'hash': torrent.info_hash.hex(),
+        'magnet': torrent.magnet_uri,
+        'main_category': torrent.main_category.name,
+        'sub_category': torrent.sub_category.name,


Could add the category IDs as well, separately or as N_N.

TheAMM · 2017-05-22T21:58:01Z

utils/api_info_v2.py

+
+info_group = parser.add_argument_group('Info options')
+
+info_group.add_argument('-q', '--query', required=True, help='Torrent by id or hash Required.')


Use a positional for this, because there's nothing else you can input - the main point is the ID/hash, just like in upload you have to have a torrent.

TheAMM · 2017-05-22T21:58:39Z

utils/api_info_v2.py

+    debug_host = args.host or os.getenv('NYAA_API_HOST')
+    api_host = (debug_host or (args.sukebei and SUKEBEI_HOST or NYAA_HOST)).rstrip('/')
+
+    api_query = args.query.lower().strip()


This is moot and if it has to be done, it should be done on the server.

TheAMM · 2017-05-22T21:59:19Z

utils/api_info_v2.py

+    matchID = re.match(ID_PATTERN, api_query)
+    matchHASH = re.match(INFO_HASH_PATTERN, api_query)
+
+    if (not matchID) and (not matchHASH):


if not (matchID or matchHASH):

TheAMM · 2017-05-22T22:00:16Z

utils/api_info_v2.py

+        print(r.text)
+    else:
+        try:
+            response = r.json()


There's no point in parsing the JSON if we're not even prettyprinting it. Just print r.text.

TheAMM

Left some notes (hopefully this'll work)

TheAMM

As asked.

TheAMM · 2017-05-24T10:08:04Z

utils/api_info_v2.py

-info_group = parser.add_argument_group('Info options')
-
-info_group.add_argument('-q', '--query', required=True, help='Torrent by id or hash Required.')
+parser.add_argument('-q', '--query', required=True, help='Torrent by id or hash Required.')


This could still be just a query with a metavar='ID_OR_HASH' - it's the only required argument for the script, so don't make the user type a useless -q. It's clear enough what the argument is.

TheAMM · 2017-05-24T10:09:52Z

nyaa/api_handler.py

+INFO_HASH_PATTERN = '^[0-9a-fA-F]{40}$'  # INFO_HASH as string
+
+
+@api_blueprint.route('/v2/info/<torrent_id_or_hash>', methods=['GET'])


Make this just a /info/, no need for v2.

TheAMM · 2017-05-24T10:11:32Z

nyaa/api_handler.py

+        'information': torrent.information,
+        'description': torrent.description,
+        'stats': {'seeders': torrent.stats.seed_count, 'leechers': torrent.stats.leech_count, 'downloads': torrent.stats.download_count},
+        'filelist': files


On another look, this could just be files, clean and simple - it's not really a list either.

TheAMM · 2017-05-24T10:11:55Z

nyaa/api_handler.py

+        'id': torrent.id,
+        'name': torrent.display_name,
+        'hash_b32': torrent.info_hash_as_b32, #as used in magnet uri
+        'hash_hex': torrent.info_hash.hex(), #as shown in torrent client


We still have a property for this, but I suppose world doesn't end at that.

TheAMM · 2017-05-24T10:14:12Z

utils/api_info_v2.py

+SUKEBEI_HOST = 'https://sukebei.nyaa.si'
+
+API_BASE = '/api'
+API_INFO = API_BASE + '/v2/info'


Remember to change this as well if you change the route.

TheAMM · 2017-05-24T10:17:39Z

Also, do rebase or otherwise resolve the merge conflict.

TheAMM · 2017-05-29T20:35:16Z

nyaa/api_handler.py

@@ -335,7 +335,11 @@ def v2_api_info(torrent_id_or_hash):
        'information': torrent.information,
        'description': torrent.description,
        'stats': {'seeders': torrent.stats.seed_count, 'leechers': torrent.stats.leech_count, 'downloads': torrent.stats.download_count},
-        'filelist': files
+        'files': files,
+        # reduce torrent flags to 0/1


Why? JSON can do boolean true/false just fine. And they are booleans.

TheAMM · 2017-05-29T20:43:03Z

utils/api_info_v2.py

-    print(r.text)
+    #print(r.text)
+
+    # For printing unicode instead of unicode escape sequences


Is the output supposed to be JSON or human readable? pprint will be just rude to the user, since they can't use the raw JSON in another script; pprint does python-repr compatible output (generally).

It's better to implement --raw like the upload api to do print(r.text) and otherwise format the output into something like

#12345: 'Some Torrent Name' uploaded by 'Username' (123Mb) Anime / English-translated - (Complete, Trusted)

etc etc.
But, you know, on second thought maybe I'll do all that myself.

shm0o · 2017-05-30T07:06:41Z

nyaa/api_handler.py

+        # reduce torrent flags to True/False
+        'is_trusted': True if torrent.trusted else False,
+        'is_complete': True if torrent.complete else False,
+        'is_remake': True if torrent.remake else False


Just for the record, you could do a bool(somevalue) call for the same effect.

shm0o · 2017-05-30T07:06:59Z

nyaa/api_handler.py

+            return_error_messages.extend(error_messages)
+
+        return flask.make_response(flask.jsonify({'Failure': return_error_messages}), 400)
+


Is this intentional/correct? 'Cause the code is unreachable as is (see the return statement a few lines above for context).

The code is indented but doesn't seem to be in the correct function.
~~I think this is~~ It appears to be a remanent of the V1 API route mistakenly re-added in a rebase/pull. ~~But I might be wrong.~~
Removed in f04c3e1#diff-81eaa857cacfa258a7a0040bbdb1dd7bL113

Yeah, that shouldn't be there in that case.

TheAMM · 2017-05-30T09:12:42Z

nyaa/api_handler.py

-        'is_trusted': 1 if torrent.trusted else 0,
-        'is_complete': 1 if torrent.complete else 0,
-        'is_remake': 1 if torrent.remake else 0
+        # reduce torrent flags to True/False


I cleaned up the properties (051f3f3) and they will be proper booleans (not just truthy), so just do torrent.trusted.

TheAMM · 2017-05-30T09:13:52Z

utils/api_info_v2.py

+            print('Info request failed:',  errors)
+            exit(1)
+        else:
+            print("Torrent #{} '{}' uploaded by '{}' ({} bytes) {} / {}\n{}".format(rj['id'], rj['name'], rj['submitter'], rj['filesize'], rj['main_category'], rj['sub_category'], rj['magnet']))


Much better!

Well now, would you look at that... I didn't know that was possible.
You can refrence the object's elements from the format replacement fields instead of adding each element to the format function.
https://docs.python.org/2/library/string.html#format-string-syntax

>>> t = {} >>> t = {'one': 1, 'two': 2} >>> '{0[one]}'.format(t) '1' >>>

Or even better, unpack the dictionary.

>>> '{one}'.format(**{'one': 1, 'two': 2}) '1'

@shm0o
~~I believe there's a vformat function (not sure about the name) so you can use it directly on the dictionary (no packing then unpacking)~~ (Irrelevant)

On this PR, I'll take what I get.

sharkykh · 2017-05-30T10:31:08Z

Don't forget to check your code's lint. https://github.com/nyaadevs/nyaa#code-quality
https://travis-ci.org/nyaadevs/nyaa/builds/237357111#L181

sharkykh · 2017-05-30T10:32:57Z

migrations/versions/3001f79b7722_add_torrents.uploader_ip.py

@@ -19,6 +19,7 @@


 def upgrade():
+


This too. There's just no reason for this file to be modified.

kyamiko · 2017-05-31T00:59:37Z

The lint script only applies to files in ./nyaa, not ./utils. This should be fixed in a different commit.

* Fix PEP8 E301 on nyaa/models.py * Add utils/ to lint checker * Run of lint.sh + manual fixes As suggested #157 (comment) * Fix backwards tick in README * Updated script * Update Travis config

TheAMM · 2017-06-03T19:25:09Z

utils/api_info_v2.py

+conn_group = parser.add_argument_group('Connection options')
+
+conn_group.add_argument('-s', '--sukebei', default=False,
+                        action='store_true', help='Upload to sukebei.nyaa.si')


'Upload to sukebei.nyaa.si' is wrong.

TheAMM · 2017-06-03T19:26:01Z

utils/api_info_v2.py

@@ -0,0 +1,98 @@
+#!/usr/bin/env python3


This is not a v2 of the info api script, this is the first one. Plain api_info.py should be fine.

TheAMM · 2017-06-03T19:27:20Z

utils/api_info_v2.py

+conn_group.add_argument('-p', '--password', help='Password')
+conn_group.add_argument('--host', help='Select another api host (for debugging purposes)')
+
+parser.add_argument('-q', '--query', required=True, help='Torrent by id or hash Required.')


This should still be just a plain parser.add_argument('hash_or_id', help='Torrent ID or hash (40-char hex string)')

TheAMM · 2017-06-03T20:56:59Z

Well, lessee if this squash will work out

TheAMM reviewed May 22, 2017

View reviewed changes

TheAMM suggested changes May 22, 2017

View reviewed changes

TheAMM suggested changes May 24, 2017

View reviewed changes

kyamiko force-pushed the master branch from 38758ee to 0e5348a Compare May 29, 2017 19:17

TheAMM reviewed May 29, 2017

View reviewed changes

shm0o reviewed May 30, 2017

View reviewed changes

TheAMM suggested changes May 30, 2017

View reviewed changes

sharkykh reviewed May 30, 2017

View reviewed changes

kyamiko force-pushed the master branch from 016d609 to a7c5309 Compare May 31, 2017 00:03

kyamiko added 7 commits May 30, 2017 21:05

API query for torrent info

6f1f13d

Updated API info for db changes

e04ada2

Restore migration script to default

2334a0b

lint

a1bed84

Requested changes for api info

17ed7cb

Updated API info, resolved conflicts

e3d0f23

Request changes, added print options

783bf0d

Cleaned up api/info

807eb9b

lint check and cleanup for api/info

0c868fd

kyamiko force-pushed the master branch from 9dd24bc to 0c868fd Compare June 3, 2017 19:14

Remove dead code from merge cleanup

909d9e8

TheAMM suggested changes Jun 3, 2017

View reviewed changes

Clean up arg parsing, requested changes

ce10872

TheAMM merged commit 570a06b into nyaadevs:master Jun 3, 2017

		INFO_HASH_PATTERN = '^[0-9a-fA-F]{40}$' # INFO_HASH as string


		@api_blueprint.route('/v2/info/<torrent_id_or_hash>', methods=['GET'])


		info_group = parser.add_argument_group('Info options')

		info_group.add_argument('-q', '--query', required=True, help='Torrent by id or hash Required.')

		return_error_messages.extend(error_messages)

		return flask.make_response(flask.jsonify({'Failure': return_error_messages}), 400)

API Info v2 #157

API Info v2 #157

Conversation

kyamiko commented May 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheAMM left a comment

Choose a reason for hiding this comment

TheAMM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheAMM commented May 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkykh May 30, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheAMM May 30, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkykh May 30, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkykh commented May 30, 2017

Choose a reason for hiding this comment

kyamiko commented May 31, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TheAMM commented Jun 3, 2017

sharkykh May 30, 2017 •

edited

TheAMM May 30, 2017 •

edited

sharkykh May 30, 2017 •

edited