Skip to content

Conversation

@viennadd
Copy link
Contributor

@viennadd viennadd commented Jun 15, 2023

Found 2 situations in special characters naming and parameter naming would cause conflicts with Python identifiers.

Case 1:

I have found that there is a dollar sign '$' character in operationId which will be converted to the name of the method, resulting invalid method name in Python

"/v1/diffs": {
	"post": {
		"tags": [
		],
		"summary": "Get Groups",
		"operationId": "get_grou##ps_$v1_ex%%ports_gr.oups__post",
		"parameters": [
		],

Case 2:

The next issue can be triggered by naming a query parameter name to one of the Python keywords.

"/v1/diffs": {
	"post": {
		"tags": [
		],
		"summary": "Get Groups",
		"operationId": "get_groups_v1_exports_groups__post",
		"parameters": [
			{
				"required": true,
				"schema": {
					"title": "from_sha of a diff",
					"type": "string"
				},
				"name": "from",
				"in": "query"
			}
		],

@MarcoMuellner
Copy link
Owner

MarcoMuellner commented Jun 15, 2023

@viennadd Can you add some tests for these cases?

The current test suite is also failing. Please take a look at that :)

@viennadd viennadd force-pushed the main branch 2 times, most recently from c5be243 to ddebb80 Compare June 15, 2023 15:23
@viennadd viennadd changed the title Filter illegal characters & keyword parameter names WIP: Filter illegal characters & keyword parameter names Jun 15, 2023
@viennadd viennadd force-pushed the main branch 2 times, most recently from 88eec1d to 488a776 Compare June 15, 2023 15:56
@viennadd viennadd changed the title WIP: Filter illegal characters & keyword parameter names Normalise names of Python identifiers Jun 15, 2023
@viennadd
Copy link
Contributor Author

@viennadd Can you add some tests for these cases?

The current test suite is also failing. Please take a look at that :)

I have added two test cases to demonstrate the issues,
Though I am not sure why the test suite are failing (

@viennadd
Copy link
Contributor Author

viennadd commented Jun 16, 2023

@viennadd Can you add some tests for these cases?

The current test suite is also failing. Please take a look at that :)

Test suite is ready now, it was my first time using codecov.io ( yep
The failure of test action was causing by the missing of CODECOV_TOKEN in my repo fork *_*

@MarcoMuellner
Copy link
Owner

Love it, thank you for the contribution!

@MarcoMuellner MarcoMuellner merged commit 0dd773d into MarcoMuellner:main Jun 16, 2023
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.

2 participants