Skip to content

Fix json serialization error in Python Driver (#1228) #1596

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.

Sign up for GitHub

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

Jump to bottom
Merged
MuhammadTahaNaveed merged 1 commit into from
Feb 14, 2024
Merged

Fix json serialization error in Python Driver (#1228) #1596

MuhammadTahaNaveed merged 1 commit into from
Feb 14, 2024

Conversation

@jrgemignani
Copy link
Contributor

@jrgemignani jrgemignani commented Feb 13, 2024

    • Added test to catch the error in toJson()

  • removed the trailing comma in _nodeToJson() node.properties part

  • removed the trailing comma in _nodeToString() node.properties part

    • Added missing ", ".join() _nodeToString() in node.properties part

Co-authored-by: Olivier oliv.maurel@gmail.com

Resolved -

Conflicts:
drivers/python/test_age_py.py

@omaurel-socha @jrgemignani
Fix json serialization error in Python Driver (apache#1228)
d4baf8c 
* * Added test to catch the error in toJson()
* removed the trailing comma in _nodeToJson() node.properties part
* removed the trailing comma in _nodeToString() node.properties part

* * Added missing ", ".join() _nodeToString() in node.properties part

---------

Co-authored-by: Olivier <oliv.maurel@gmail.com>

Resolved -

Conflicts:
	drivers/python/test_age_py.py
@github-actions github-actions bot added the PG13 PostgreSQL13 label Feb 13, 2024
@MuhammadTahaNaveed
Copy link
Member

MuhammadTahaNaveed commented Feb 14, 2024

@jrgemignani Why does this PR show 428 additions when the same patch on PG15 is showing 61 additions? looks like its due to formatting, but was this intentional?

@rafsun42
Copy link
Member

rafsun42 commented Feb 14, 2024

@MuhammadTahaNaveed Yes, it seems intentional. Some trailing spaces were removed. GitHub diff tool did not do a good job showing that. But, I could see that in vscode diff.

@jrgemignani
Copy link
Contributor Author

jrgemignani commented Feb 14, 2024

@jrgemignani Why does this PR show 428 additions when the same patch on PG15 is showing 61 additions? looks like its due to formatting, but was this intentional?

@MuhammadTahaNaveed The file drivers/python/test_age_py.py differs from the higher versions (and this applies to the lower versions too) in having a ^M embedded at the end of the line and this change removes those.

@MuhammadTahaNaveed
Copy link
Member

MuhammadTahaNaveed commented Feb 14, 2024 •
edited
Loading

@rafsun42 @jrgemignani oh okay... Thanks for the clarification...

@MuhammadTahaNaveed MuhammadTahaNaveed merged commit 468fb29 into apache:PG13 Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MuhammadTahaNaveed MuhammadTahaNaveed MuhammadTahaNaveed approved these changes

@Zainab-Saad Zainab-Saad Zainab-Saad approved these changes

@dehowef dehowef Awaiting requested review from dehowef

@rafsun42 rafsun42 Awaiting requested review from rafsun42

Assignees

No one assigned

Labels

PG13 PostgreSQL13

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jrgemignani @MuhammadTahaNaveed @rafsun42 @Zainab-Saad @omaurel-socha