See:

OpenTTD/BaNaNaS-staging@0c24b2b

There was a change and it was reverted within the graceperiod. In result, an empty commit.

Please fix this.

Request:
GET /package///
with an unknown unique-id or upload-date.

Expected: Response 404

Currently: Response 500

I just want to close it

Based on https://www.tt-forums.net/viewtopic.php?p=1234714#p1234714

A ZIP file with a music set uploaded to BaNaNaS had the directory in the root of the ZIP file stripped, so all the contents are at the root of the generated TAR file. This causes OpenTTD to discard the downloaded file (see OpenTTD/OpenTTD#8283) as invalid, so while the download at first appears to have worked, it actually failed.

Currently you can only upload new scenarios/heightmaps (as in, get a new unique-id), but you can never update an existing.

There is no technical reason for this, other than we didn't have the code for it yet.

Request:
GET /package/<content-type>/<unique-id>
with an unknown unique-id.

Expected: Response 404

Currently: Response 200 with empty json {}.

For Game Scripts and AIs this is kinda the only way to upload anything with a folder via the webinterface. The CLI does support uploading files via folders already, but this would just be a nice addition.

It would be nice to show in the frontend what the license is (if custom), or show the readme or changelog.

Technically, this is a bit tricky, as these files (after upload) are only available inside the tarball. Most likely it is a good idea to store those files separatly on S3, so it is easy to fetch them. Not a trivial issue to fix for sure.

Package with "name" and "version" of non-ASCII characters will have a filename "-.tar.gz".

This is because _safe_name() removes any non-ASCII (well, give or take).

We should refuse these uploads, as this is most likely not what the uploaded intended.

Some content is approaching a size of 1 GB.
With some connections a upload timeout of 15 minutes is rather harsh.

It's also likely that people would start their GB upload, and go for a walk.

So I suggest to make upload-tokens expire together with the session, and not on their own.

Currently you cannot modify the authors of a package, except by making a Pull Request on GitHub. Adding this functionality would be helpful, although it is not heavily used.

When uploading an update to an existing package, the front-end makes the same assumption as when editing meta data. "No name" means "name from package".
It looks like the API agrees in the validation step, since it does not complain about missing stuff in the GET /new-package/.

However, .../publish gives 500:

2020-04-25 21:48:46 ERROR Error handling request
Traceback (most recent call last):
File "/usr/local/lib/python3.8/site-packages/aiohttp/web_protocol.py", line 418, in start
resp = await task
File "/usr/local/lib/python3.8/site-packages/aiohttp/web_app.py", line 458, in _handle
resp = await handler(request)
File "/code/bananas_api/web_routes/new.py", line 194, in new_publish
publish_session(session)
File "/code/bananas_api/new_upload/session.py", line 232, in publish_session
create_tarball(session)
File "/code/bananas_api/new_upload/session_publish.py", line 112, in create_tarball
tar_path = _safe_name(session["name"]) + "-" + _safe_name(session["version"])
KeyError: 'name'
2020-04-25 21:48:46 INFO 172.17.0.1 [25/Apr/2020:21:48:46 +0000] "POST /new-package/3a6ad88d8398ed157f8e8af1f9c5157c/publish HTTP/1.1" 500 244 "-" "python-requests/2.23.0"

Currently you can make a heightmap depend on an AI, which makes very little sense.

The list should be like this:

• ai, ai-library: only allow ai-library
• base-graphics, base-music, base-sounds: no dependencies allowed
• game-script, game-script-library: only allow game-script-library
• heightmap, newgrf: no dependencies allowed
• scenario: only allow newgrf, ai and game-script

Currently we have AIs that depend on a scenario. There is a case for this, but not sure it is a solid one.

If you run the bananas-api as suggested in the readme using:

docker run --rm -p 127.0.0.1:8080:80 -p 127.0.0.1:1080:1080 -v "${BANANAS_COMMON}/local_storage:/code/local_storage" -v "${BANANAS_COMMON}/BaNaNaS:/code/BaNaNaS" openttd/bananas-api:local

Upload of a newgrf fails with the following error:

OSError: [Errno 18] Invalid cross-device link

The issue is that the temporary upload path and the local_storage path are not on the same physical device (one is inside the virtual file system of the docker container, while the other is on a volume mounted on the host).

According to the documentation I have found, shutil.move is the correct function to use if you are moving between file systems.

It is now possible to rename your package, and as such, people are renaming multiple packages of theirs to the same name.

I have no real problems with this, as the system considers the name a piece of metadata, but it turns out the OpenTTD client has a different vision on this.

When downloading a piece of content with the same name, it appears to be overwriting the existing file. This of course can be considered a bug on its own, but there is also not really a resolution for the client.

I want to suggest to rename the packages to <uniqueid>-<name>-<version>. This is still human-readable, and always unique.

Idea to enable bots/GH actions to upload packages on releases.

• Owner generates a pair of public/private keys.
• Public key is stored in authors.yaml.
• Private key is kept by uploading bots as secret.
• bananas-cli and api perform a handshake for authentication.

See

OpenTTD/BaNaNaS-staging@e66dabd

There should only be one notice that a package changed.

Currently this has to be hardocded in all the clients. Please take control of this, and make it an endpoint.

It's easy to forget these files, and in-game they are as vital as the description.

So the upload validation should warn, if no readme and/or changelog is part of the upload, and recommend adding them.

I uploaded an update to an existing package, with a custom name/tags/description/...

None of the entered information was stored.

Currently the latest upload is always "new-games", and anything else is "savegames-only". This is not technically required by anything, it was just easier to implement.

Please create an additional to the API that allows to set any version to "new-games" and "savegames-only". Possibly it would be good if two versions are "new-games" it is enforced that their min/max compatability-versions don't overlap (as that would make no sense).

For new-games, anyone should be able to get the download URLs.
For savegames-only, only the author should be able to get the download URL.

Most likely this is not the case, but we really should do this.

We know this information, we already extract it from the files, but we don't do anything with it.

Would be a nice additional, and a huge QoL for many users.

Sentry Issue: BANANAS-API-D

ReadError: file could not be opened successfully
(1 additional frame(s) were not displayed)
...
  File "aiohttp/web_app.py", line 458, in _handle
    resp = await handler(request)
  File "bananas_api/web_routes/new.py", line 92, in tusd_handler
    add_file(
  File "bananas_api/new_upload/session.py", line 236, in add_file
    session["files"].extend(extract_tarball(new_file))
  File "bananas_api/new_upload/extract.py", line 79, in extract_tarball
    with tarfile.open(file_info["internal_filename"]) as tar:
  File "tarfile.py", line 1604, in open
    raise ReadError("file could not be opened successfully")

Error handling request

Putting yourself as dependency is valid.

This of course is wrong, and should not be possible. This should be filtered on unique_id already, you also aren't allowed to set dependencies on an older version of your own content.

We currently list CC v3, but v4 is out already.

For example, via GitHub Teams. Just any form that multiple people can create a single content, without them sharing a password.

Possibly /user could return

• "review-url": "https://github.com/settings/connections/applications/:client_id" (with client-id filled)

Then the front-end can display this link on the manager site or similar.

Changes are not being committed immediately, so add an API endpoint that shows what changes are pending. Possibly add a button to commit the changes immediately.

Validation of big files (I tried a NewGRF with 850 MB) takes several seconds (probably computing MD5). The validation is repeated for every PUT/GET, even when only updating meta-data like description.

Please cache the validation result of uploaded files, if they do not change.

This information is already inside these files, and we already extract it. Adding this would make uploads a lot easier for many users, so a huge QoL.

With this, we should also restrict manual editing of dependencies to only AI/AI Library/GS/GS Library.

Currently "url" field can be anything. Validate that it starts with http/https.

Currently you could check the commit history; but having it per entry would be much cleaner.

Set version members to "" should delete them (so that package data becomes active).
Not setting the fieldis used for "unchanged".