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

Toolchain v6 #556

Open
wants to merge 39 commits into
base: main
Choose a base branch
from
Open

Toolchain v6 #556

wants to merge 39 commits into from

Conversation

mattkjames7
Copy link

@mattkjames7 mattkjames7 commented Mar 7, 2025

Description

Building and testing using toolchain-v6

Pull request type

  • Bugfix
  • Algorithm/Module
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Related issues

Delete if this PR doesn't resolve any issues. Link the issue if it does.

######################################

Reviewer checklist (the reviewer checks this part)

Module/Algorithm

  • Core algorithm/module implementation
  • Query module implementation
  • Tests provided (unit / e2e)
  • Code documentation
  • README short description

Documentation checklist

  • Add the documentation label tag
  • Add the bug / feature label tag
  • Add the milestone for which this feature is intended
    • If not known, set for a later milestone
  • Write a release note, including added/changed clauses
    • [Release note text]
  • Link the documentation PR here
    • [Documentation PR link]
  • Tag someone from docs team in the comments

Copy link

sonarqubecloud bot commented Mar 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
4 Security Hotspots
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@mattkjames7 mattkjames7 marked this pull request as ready for review March 10, 2025 08:50
Copy link
Contributor

@imilinovic imilinovic left a comment

Choose a reason for hiding this comment

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

Overall looks good to me, few comments.

@@ -70,7 +70,7 @@ jobs:
mage_build_target: "${{ matrix.build_target }}"
mage_build_type: "${{ matrix.build_type }}"
memgraph_version: "${{ github.event.inputs.memgraph_version }}"
memgraph_download_link: "${{ needs.SetupNames.outputs.download_link_base }}${{ matrix.arm_ext }}${{ matrix.rwid_ext }}/${{ needs.SetupNames.outputs.dowlnoad_binary_name_base }}_${{ matrix.build_arch }}64.deb"
memgraph_download_link: "${{ needs.SetupNames.outputs.download_link_base }}${{ matrix.arm_ext }}${{ matrix.rwid_ext }}/${{ needs.SetupNames.outputs.dowlnoad_binary_name_base }}_${{ matrix.build_arch }}.deb"
Copy link
Contributor

Choose a reason for hiding this comment

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

dowlnoad_binary_name_base
Typo, if its not inside this code ignore.

Comment on lines +67 to +68
# - name: Set up QEMU
# uses: docker/setup-qemu-action@v3
Copy link
Contributor

Choose a reason for hiding this comment

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

remove?

echo "Using Dockerfile: $DOCKERFILE"

# Set up Docker Buildx
docker buildx create --use || true
Copy link
Contributor

Choose a reason for hiding this comment

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

|| true is so if build exists it continues?

Comment on lines +141 to +171
# echo "Start Neo4j..."
# docker run --rm \
# --name "$NEO4J_CONTAINER" \
# --network "$MEMGRAPH_NETWORK" \
# -p 7474:7474 \
# -d \
# -v "$HOME/neo4j/plugins:/plugins" \
# --env NEO4J_AUTH=none \
# -e NEO4J_apoc_export_file_enabled=true \
# -e NEO4J_apoc_import_file_enabled=true \
# -e NEO4J_apoc_import_file_use__neo4j__config=true \
# -e NEO4J_PLUGINS='["apoc"]' neo4j:5.10.0

# echo "Waiting for Neo4j to start..."
# counter=0
# timeout=30
# while ! curl --silent --fail http://localhost:7474; do
# sleep 1
# counter=$((counter+1))
# if [ $counter -gt $timeout ]; then
# echo "Neo4j failed to start in $timeout seconds"
# exit 1
# fi
# done
# echo "Neo4j is up and running."

# echo "Running e2e correctness tests..."
# docker exec -i -u memgraph "$MAGE_CONTAINER" bash -c "cd /mage && python3 test_e2e_correctness.py --memgraph-port $MEMGRAPH_PORT --neo4j-port $NEO4J_PORT --neo4j-container $NEO4J_CONTAINER"

# echo "Stopping Neo4j..."
# docker stop "$NEO4J_CONTAINER"
Copy link
Contributor

Choose a reason for hiding this comment

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

remove


# Cleanup
docker stop "$MAGE_CONTAINER" || true
#docker rmi memgraph-mage:$build_target || true
Copy link
Contributor

Choose a reason for hiding this comment

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

debug line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants