Skip to content

remote.nextstrain_dot_org: Handle 400 Bad Request errors#238

Merged
tsibley merged 1 commit intomasterfrom
trs/remote/nextstrain.org/bad-requests
Dec 1, 2022
Merged

remote.nextstrain_dot_org: Handle 400 Bad Request errors#238
tsibley merged 1 commit intomasterfrom
trs/remote/nextstrain.org/bad-requests

Conversation

@tsibley
Copy link
Contributor

@tsibley tsibley commented Nov 23, 2022

This helps in particular with user-caused bad requests, such as trying to upload to dataset URLs which include underscores instead of slashes. It will also let us use 400 responses more widely on the server side since we know the CLI user will now see the error message.

Related-to: nextstrain/nextstrain.org#611

Testing

Example of the error message:

$ NEXTSTRAIN_DOT_ORG=http://localhost:5000 nextstrain remote upload groups/test/foo_bar foo_bar
.json
Uploading foo_bar.json as http://localhost:5000/groups/test/foo_bar
Error: The remote server rejected our request:

  Resource (e.g. dataset and narrative) paths may not include
  underscores (_); use slashes (/) instead

This may indicate a problem that's fixable by you, or it
may be a bug somewhere that needs to be fixed by the
Nextstrain developers.

If you're unable to address the problem, please open a new
issue at <https://github.com/nextstrain/cli/issues/new/choose>
and include the complete output above and the command you
were running.
  • Checks pass
  • Manual testing of 400 with decodable JSON body
  • Manual testing of 400 with bad JSON body

@tsibley
Copy link
Contributor Author

tsibley commented Nov 23, 2022

Ugh, CI is failing (again) for unrelated reasons (again). This time, it appears to be a recent release of flake8 with more stringent config parsing. Will fix on another PR (again).

@tsibley tsibley mentioned this pull request Nov 23, 2022
1 task
This helps in particular with user-caused bad requests, such as trying
to upload to dataset URLs which include underscores instead of slashes.
It will also let us use 400 responses more widely on the server side
since we know the CLI user will now see the error message.

Related-to: <nextstrain/nextstrain.org#611>
@tsibley tsibley force-pushed the trs/remote/nextstrain.org/bad-requests branch from 2cbefb1 to 08fa0c3 Compare November 23, 2022 23:33
@tsibley
Copy link
Contributor Author

tsibley commented Nov 23, 2022

Rebased to fix CI issues.

@tsibley tsibley requested a review from a team December 1, 2022 01:32
@tsibley tsibley merged commit 869759b into master Dec 1, 2022
@tsibley tsibley deleted the trs/remote/nextstrain.org/bad-requests branch December 1, 2022 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

2 participants