Skip to content

fixed the deprecated keywords in the neo4j queries - #159

Merged
m-appel merged 8 commits into
InternetHealthReport:mainfrom
ronibhakta1:fixed/154/Replaced-use-of-id-with-elementId
Dec 9, 2024
Merged

fixed the deprecated keywords in the neo4j queries#159
m-appel merged 8 commits into
InternetHealthReport:mainfrom
ronibhakta1:fixed/154/Replaced-use-of-id-with-elementId

Conversation

@ronibhakta1

@ronibhakta1 ronibhakta1 commented Dec 4, 2024

Copy link
Copy Markdown
Contributor

@m-appel check this out i have made the asked changes...
closes #154

Description

id() changed with elementId() in neo4j with new required documentation elementId()

Motivation and Context

How Has This Been Tested?

pre-commit checks

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

Comment thread iyp/__init__.py Outdated
def load_cached_object(self, object_name: str):
cache_file = f'{self.cache_file_prefix}{object_name}{self.cache_file_suffix}'
cache_file = f'{self.cache_file_prefix}{
object_name}{self.cache_file_suffix}'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is broken now and causes a syntax error.

@m-appel

m-appel commented Dec 4, 2024

Copy link
Copy Markdown
Member

Hi, I'm not sure what kind of code formatter you use, but please don't introduce unnecessary formatting changes as it crowds the PR. Our line length (specified in .flake8) is 120 characters.
The formatter also broke one line, which is why the pre-commit checks are failing.

So please only change the requested fields and test the functions. The testing part is the reason this is an issue and not just a simple Search + Replace, since it requires running a neo4j instance and confirming that the functions still behave the same.

@m-appel
m-appel self-requested a review December 4, 2024 06:31
@ronibhakta1

Copy link
Copy Markdown
Contributor Author

Hi, I'm not sure what kind of code formatter you use, but please don't introduce unnecessary formatting changes as it crowds the PR. Our line length (specified in .flake8) is 120 characters.
The formatter also broke one line, which is why the pre-commit checks are failing.

So please only change the requested fields and test the functions. The testing part is the reason this is an issue and not just a simple Search + Replace, since it requires running a neo4j instance and confirming that the functions still behave the same.

I will take a look at it, I'm trying to understand the whole product itself.

spacing fix
@ronibhakta1

Copy link
Copy Markdown
Contributor Author

Hi, I'm not sure what kind of code formatter you use, but please don't introduce unnecessary formatting changes as it crowds the PR. Our line length (specified in .flake8) is 120 characters. The formatter also broke one line, which is why the pre-commit checks are failing.

So please only change the requested fields and test the functions. The testing part is the reason this is an issue and not just a simple Search + Replace, since it requires running a neo4j instance and confirming that the functions still behave the same.

i have done the fix in the files. can you help me with running a neo4j instance and confirming that the functions still behave the same.

@m-appel

m-appel commented Dec 6, 2024

Copy link
Copy Markdown
Member

Hey, since this has become somewhat urgent, I am testing this myself.
In general, to get a database up and running, you should look at the main README.
To test the behavior, I am just running versions of the old and new queries against the webinterface and see if they behave the same.

@ronibhakta1

Copy link
Copy Markdown
Contributor Author

,

Hey thanks a lot, I'm new to open source and contributing to many organisations. And due to different projects it's taking time to understand their projects. I have checked the issue I was facing on my end was due to vs code idk why it was making a mess on my Mac. Anyways thanks again I will get things done next time :)

@m-appel

m-appel commented Dec 6, 2024

Copy link
Copy Markdown
Member

No worries, I still welcome you to try and getting IYP up and running and try it out.
I would have given you more time, but we just updated our neo4j version and are now getting spammed with "id() is deprecated" messages, so I want to merge this asap :)

@ronibhakta1

ronibhakta1 commented Dec 6, 2024 via email

Copy link
Copy Markdown
Contributor Author

@m-appel
m-appel merged commit 4985ad7 into InternetHealthReport:main Dec 9, 2024
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.

Replace use of id() with elementId()

3 participants