Skip to content
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.

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

feature/mx-1702 improve errors and logs #189

Merged
merged 7 commits into from
Nov 8, 2024

Conversation

cutoffthetop
Copy link
Contributor

@cutoffthetop cutoffthetop commented Oct 31, 2024

PR Context

  • some additional prep for mx-1702

Changes

  • mute warnings about labels used in queries but missing in graph
  • split up search_merged_items_in_graph for better readability
  • update cypher queries to use CALL clauses with correct variable scope
  • BREAKING: drop support for neo4j server version 5.6 and lower

Signed-off-by: Nicolas Drebenstedt <897972+cutoffthetop@users.noreply.github.com>
Copy link
Contributor

@rababerladuseladim rababerladuseladim left a comment

Choose a reason for hiding this comment

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

During local testing I get a bunch of errors like this: FAILED tests/graph/test_connector.py::test_fetch_rule_items_empty - neo4j.exceptions.ConfigurationError: Notification filtering is not supported for the Bolt Protocol Version(5, 0). Server Agent 'None'

Do I need to update my neo4j installation? And if yes: Could you communicate this in the dev channel and update our development setup guidelines?

mex/backend/graph/cypher/merge_item.cql Outdated Show resolved Hide resolved
mex/backend/graph/cypher/merge_edges.cql Outdated Show resolved Hide resolved
@rababerladuseladim
Copy link
Contributor

And while we're at it: What do you think about expanding the setup instructions in the README with the neo4j installation?

@cutoffthetop
Copy link
Contributor Author

Do I need to update my neo4j installation? And if yes: Could you communicate this in the dev channel and update our development setup guidelines?

the notification settings were introduced in neo4j 5.7, so if you were below that you'd had to upgrade. i added a BREAKING point to the changelog, and will post a reminder upon release.
i also added a bulletpoint on upgrading to our guidelines.

And while we're at it: What do you think about expanding the setup instructions in the README with the neo4j installation?

our setup guide is quite specific to our needs, so i'd leave that in our internal docs. however, i added a short and more general version of it to the readme.

Copy link
Contributor

@rababerladuseladim rababerladuseladim left a comment

Choose a reason for hiding this comment

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

All good, merge away!

@cutoffthetop cutoffthetop merged commit 0956050 into main Nov 8, 2024
4 checks passed
@cutoffthetop cutoffthetop deleted the feature/mx-1702-errors-and-logs branch November 8, 2024 13:49
rababerladuseladim added a commit to robert-koch-institut/mex-editor that referenced this pull request Nov 14, 2024
# PR Context
- this PR only resolves Links, Texts and Enums, but does not resolve
Identifiers yet
- additionally cleans up the user interface and code structure a bit
- needs robert-koch-institut/mex-backend#187 and
robert-koch-institut/mex-backend#189

# Changes
- upgrade mex-common and model dependencies to v3
- overhaul and simplify margins and spaces
- move transform_models_to_fields from State to transform module
- use dedicated backend connector methods for edit and search
- use same rendering components for edit and search pages
- pop toasts when backend connector encounters errors
- scroll to top when pagination is triggered

# Fixed
- fix routing issues by moving the refresh handlers section.from
on_mount to page.on_load

---------

Signed-off-by: Nicolas Drebenstedt <897972+cutoffthetop@users.noreply.github.com>
Co-authored-by: Franziska Diehr <DiehrF@rki.de>
Co-authored-by: RKI | Metadata Exchange <121876825+RKIMetadataExchange@users.noreply.github.com>
Co-authored-by: rababerladuseladim <rababerladuseladim@users.noreply.github.com>
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