Skip to content

Commit

Permalink
fix: evaluate profile_default (#1767)
Browse files Browse the repository at this point in the history
  • Loading branch information
jhaeu authored Apr 19, 2024
2 parents 9472c65 + e63588e commit ae528d3
Show file tree
Hide file tree
Showing 20 changed files with 219 additions and 100 deletions.
4 changes: 3 additions & 1 deletion .github/utils/yml_config_to_ors_config_conversion.sh
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ echo "- Uncomment ors, engine and source_file"
sed -i -e '/^#ors:/s/^#//' -e '/^#.*engine:/s/^#//' -e '/^#.*source_file:/s/^#//' "$output_file"

echo "- Uncomment subsequent lines for profiles.car.enabled in ors.engine"
sed -i -e '/^# profiles:/,/^# enabled:/ s/^#//' "$output_file"
sed -i -e '/^# profiles:/s/^#//' "$output_file"
sed -i -e '/^# car:.*/s/^#//' "$output_file"
sed -i -e '/^# enabled: true/s/^#//' "$output_file"

echo "Parsing complete. Result saved to $output_file"
4 changes: 2 additions & 2 deletions .github/utils/yml_config_validation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@ yq 'true' $input_file /dev/null || exit 1
echo "- checking if ors.engine.source_file exists and has 'source_file' property"
yq --exit-status '.ors.engine | has("source_file")' $input_file > /dev/null || exit 1
# For profiles section for car with enabled using yq and contains
echo "- checking if ors.engine.profiles.car exists and has 'enabled' property"
yq --exit-status '.ors.engine.profiles.car | has("enabled")' $input_file > /dev/null || exit 1
#echo "- checking if ors.engine.profiles.car exists and has 'enabled' property"
#yq --exit-status '.ors.engine.profiles.car | has("enabled")' $input_file > /dev/null || exit 1
2 changes: 1 addition & 1 deletion .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ jobs:
strategy:
matrix:
runtype: [ '-m' , '-j' ]
testgroup: [ build-all-graphs*, arg-overrides*, check*, config*, lookup*, missing*, specify* ]
testgroup: [ build-all-graphs*, arg-overrides*, check*, config-json*, config-yml*, lookup*, missing*, ors-config*, specify-json*, specify-yml*, profile-default* ]
steps:
- uses: actions/checkout@v4
with:
Expand Down
36 changes: 31 additions & 5 deletions .integration-scenarios/debian-12-jar-mvn/files/testfunctions.sh
Original file line number Diff line number Diff line change
Expand Up @@ -85,20 +85,46 @@ function expectOrsStartupFails() {
fi
}

function assertSortedWordsEquals() {
expected=$1
received=$2
sorted_expected=$(echo "$expected" | tr ' ' '\n' | sort | tr '\n' ' ')
sorted_received=$(echo "$received" | tr ' ' '\n' | sort | tr '\n' ' ')
assertEquals "$sorted_expected" "$sorted_received"
}

function assertEquals() {
expected=$1
received=$2
check=$3
if [ -n "$check" ]; then checkMsg="Checking '$check': "; fi
if [ "$expected" != "$received" ]; then
echo -e "${FG_RED}ASSERTION ERROR:${N}"
echo -e "${FG_RED}ASSERTION ERROR:${N} ${checkMsg}"
echo -e "expected: '${FG_GRN}${expected}${N}'"
echo -e "received: '${FG_RED}${received}${N}'"
exit 1
else
echo -e "${FG_GRN}received '$received' as expected${N}"
exit 0
echo -e "${FG_GRN}${checkMsg}Received '${received}' as expected${N}"
fi
}

function assertContains() {
expected=$1
received=$2
num=$(echo "${received}" | grep -c "${expected}")
if [[ $num -eq 0 ]]; then
echo -e "${FG_RED}ASSERTION ERROR:${N}: '${expected}' not contained as expected${N} $num"
exit 1
else
echo -e "${FG_GRN}'${expected}' is contained as expected${N}"
fi
}

function requestStatusString() {
port=$1
echo $(curl --silent $(getOrsUrl $port)/status | jq . )
}

function requestEnabledProfiles() {
port=$1
echo $(curl --silent $(getOrsUrl $port)/status | jq -r '.profiles[].profiles')
Expand Down Expand Up @@ -153,7 +179,7 @@ function prepareTest() {
if [ -z "$IMAGE" ]; then printError "missing param 2: docker image"; exit 1; fi

CONTAINER=${runType}-$(removeExtension "$(basename $script)")
HOST_PORT=$(findFreePort 8082)
HOST_PORT=$(findFreePort 8083)

mkdir -p ~/.m2
M2_FOLDER="$(realpath ~/.m2)"
Expand Down Expand Up @@ -183,5 +209,5 @@ function makeTempFile() {

function deleteTempFiles() {
script=$1
rm "${TESTROOT}/tmp/${script}".*
rm "${TESTROOT}/tmp/${script}"*
}
15 changes: 10 additions & 5 deletions .integration-scenarios/debian-12-jar-mvn/run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ mvn=0
verbose=0
pattern=""

function printCliHelp() { # TODO adapt
function printCliHelp() {
echo -e "\
${B}$SCRIPT${N} - run ors tests in containers
Expand Down Expand Up @@ -69,12 +69,15 @@ function runTest() {
else
$testscript "${runType}" "${imageName}" 1>/dev/null 2>&1
fi

if (($?)); then
testStatus=$?
if [ $testStatus -eq 1 ]; then
hasErrors=1
((failed++))
echo -e "${FG_RED}${B}failed${N}"
(($failFast)) && exit 1
elif [ $testStatus -eq 2 ]; then
((skipped++))
echo -e "${FG_ORA}${B}skipped${N}"
else
((passed++))
echo -e "${FG_GRN}passed${N}"
Expand Down Expand Up @@ -142,6 +145,7 @@ mkdir -p "${TESTROOT}/graphs_volume"

hasErrors=0
passed=0
skipped=0
failed=0

for word in $pattern; do
Expand All @@ -152,8 +156,9 @@ for word in $pattern; do
done

(($passed)) && passedText=", ${FG_GRN}${B}${passed} passed${N}"
(($skipped)) && skippedText=", ${FG_ORA}${B}${skipped} skipped${N}"
(($failed)) && failedText=", ${FG_RED}${B}${failed} failed${N}"

total=$(($passed + $failed))
echo -e "${FG_BLU}$(date +%Y-%m-%dT%H:%M:%S)${N} ${B}done, ${total} test$( (($total-1)) && echo "s") executed${passedText}${failedText}"
total=$(($passed + $skipped + $failed))
echo -e "${FG_BLU}$(date +%Y-%m-%dT%H:%M:%S)${N} ${B}done, ${total} test$( (($total-1)) && echo "s") executed${passedText}${skippedText}${failedText}"
exit $hasErrors
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ awaitOrsReady 60 "${HOST_PORT}"
profiles=$(requestEnabledProfiles ${HOST_PORT})
cleanupTest

assertEquals 'driving-hgv driving-car' "${profiles}"
assertSortedWordsEquals 'driving-hgv driving-car' "${profiles}"
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,4 @@ awaitOrsReady 300 "${HOST_PORT}"
profiles=$(requestEnabledProfiles ${HOST_PORT})
cleanupTest

assertEquals "foot-walking wheelchair foot-hiking public-transport cycling-electric cycling-mountain driving-car driving-hgv cycling-regular cycling-road" "${profiles}"
assertSortedWordsEquals "foot-walking wheelchair foot-hiking public-transport cycling-electric cycling-mountain driving-car driving-hgv cycling-regular cycling-road" "${profiles}"
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ source $TESTROOT/files/testfunctions.sh
source $TESTROOT/files/test.conf
prepareTest $(basename $0) $*

if [ "$runType" = "mvn" ]; then
echo "skipping - mvn does not support env variables with dot notation"
exit 2;
fi

# Even if no yml config file is present, the ors is runnable
# if at least one routing profile is enabled with a environment variable.
podman run --replace --name "${CONTAINER}" -p "${HOST_PORT}":8082 \
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/usr/bin/env bash

TESTROOT="$( cd "$(dirname "$0")"/.. >/dev/null 2>&1 ; pwd -P )"
source $TESTROOT/files/testfunctions.sh
source $TESTROOT/files/test.conf
prepareTest $(basename $0) $*

configCar=$(makeTempFile $(basename $0) "\
ors:
engine:
source_file: ors-api/src/test/files/heidelberg.osm.gz
profiles:
car:
enabled: true")

# The profile configured as run argument should be preferred over environment variable.
# The default yml file should not be used when ORS_CONFIG_LOCATION is set,
# even if the file does not exist. Fallback to default ors-config.yml is not desired!
podman run --replace --name "${CONTAINER}" -p "${HOST_PORT}":8082 \
-v "${M2_FOLDER}":/root/.m2 \
-v "${TESTROOT}/graphs_volume":"${CONTAINER_WORK_DIR}/graphs" \
-v "${configCar}":${CONTAINER_WORK_DIR}/ors-config.yml \
--env ORS_CONFIG_LOCATION=${CONTAINER_WORK_DIR}/nonexisting.yml \
"local/${IMAGE}:latest" \
$(getProgramArguments ${runType} ${CONTAINER_WORK_DIR}/config-car.yml) &


# expect process finished timout
res=$(expectOrsStartupFails 60 "$CONTAINER" )
# stop container if was not finished
cleanupTest

assertEquals "terminated" "$res"
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
#!/usr/bin/env bash

TESTROOT="$( cd "$(dirname "$0")"/.. >/dev/null 2>&1 ; pwd -P )"
source $TESTROOT/files/testfunctions.sh
source $TESTROOT/files/test.conf
prepareTest $(basename $0) $*

configPT=$(makeTempFile $(basename $0) "\
ors:
engine:
source_file: ors-api/src/test/files/heidelberg.osm.gz
profile_default:
enabled: false
profiles:
public-transport:
gtfs_file: ors-api/src/test/files/vrn_gtfs_cut.zip
")

# When profiles are not enabled as default and none is explicitly enabled,
# then ORS should not start up
podman run --replace --name "${CONTAINER}" -p "${HOST_PORT}":8082 \
-v "${M2_FOLDER}":/root/.m2 \
-v "${TESTROOT}/graphs_volume":"${CONTAINER_WORK_DIR}/graphs" \
-v "${configPT}":"${CONTAINER_WORK_DIR}/ors-config.yml" \
"local/${IMAGE}:latest" &

res=$(expectOrsStartupFails 300 "$CONTAINER" )
cleanupTest

assertEquals "terminated" "$res"
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/usr/bin/env bash

TESTROOT="$( cd "$(dirname "$0")"/.. >/dev/null 2>&1 ; pwd -P )"
source $TESTROOT/files/testfunctions.sh
source $TESTROOT/files/test.conf
prepareTest $(basename $0) $*

configPT=$(makeTempFile $(basename $0) "\
ors:
engine:
source_file: ors-api/src/test/files/heidelberg.osm.gz
profile_default:
enabled: true
profiles:
public-transport:
gtfs_file: ors-api/src/test/files/vrn_gtfs_cut.zip
")

# When profile_default.enabled: true
# then all profiles should be enabled
podman run --replace --name "${CONTAINER}" -p "${HOST_PORT}":8082 \
-v "${M2_FOLDER}":/root/.m2 \
-v "${TESTROOT}/graphs_volume":"${CONTAINER_WORK_DIR}/graphs" \
-v "${configPT}":"${CONTAINER_WORK_DIR}/ors-config.yml" \
"local/${IMAGE}:latest" &

awaitOrsReady 300 "${HOST_PORT}"
profiles=$(requestEnabledProfiles ${HOST_PORT})
cleanupTest

assertSortedWordsEquals "foot-walking wheelchair foot-hiking public-transport cycling-electric cycling-mountain driving-car driving-hgv cycling-regular cycling-road" "${profiles}"
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
#!/usr/bin/env bash

TESTROOT="$( cd "$(dirname "$0")"/.. >/dev/null 2>&1 ; pwd -P )"
source $TESTROOT/files/testfunctions.sh
source $TESTROOT/files/test.conf
prepareTest $(basename $0) $*

configPT=$(makeTempFile $(basename $0) "\
ors:
engine:
source_file: ors-api/src/test/files/heidelberg.osm.gz
profile_default:
maximum_distance: 111111
maximum_distance_dynamic_weights: 111111
maximum_distance_avoid_areas: 111111
maximum_waypoints: 11
profiles:
car:
enabled: true
maximum_waypoints: 55
")

podman run --replace --name "${CONTAINER}" -p "${HOST_PORT}":8082 \
-v "${M2_FOLDER}":/root/.m2 \
-v "${TESTROOT}/graphs_volume":"${CONTAINER_WORK_DIR}/graphs" \
-v "${configPT}":"${CONTAINER_WORK_DIR}/ors-config.yml" \
"local/${IMAGE}:latest" &

awaitOrsReady 300 "${HOST_PORT}"
statusString=$(requestStatusString ${HOST_PORT})
cleanupTest

assertEquals "111111" "$(echo $statusString | jq -r '.profiles."profile 1".limits.maximum_distance')" "maximum_distance"
assertEquals "55" "$(echo $statusString | jq -r '.profiles."profile 1".limits.maximum_waypoints')" "maximum_waypoints"
assertEquals "111111" "$(echo $statusString | jq -r '.profiles."profile 1".limits.maximum_distance_dynamic_weights')" "maximum_distance_dynamic_weights"
assertEquals "111111" "$(echo $statusString | jq -r '.profiles."profile 1".limits.maximum_distance_avoid_areas')" "maximum_distance_avoid_areas"
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ RELEASING:

### Fixed
- preparation mode exiting with code 0 on fail ([#1772](https://github.com/GIScience/openrouteservice/pull/1772))
- some more properties can be defined in a user's ors-config.yml/env ors.engine.profile_default without side effects (Issue [#1762](https://github.com/GIScience/openrouteservice/issues/1762))

## [8.0.0] - 2024-03-21
### Added
Expand Down
40 changes: 24 additions & 16 deletions docs/run-instance/configuration/ors/engine/profiles.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,30 @@

The profiles object contains key-object-pairs for each profile you are using.

Available profiles are:
- `car`
- `hgv`
- `bike-regular`
- `bike-mountain`
- `bike-road`
- `bike-electric`
- `walking`
- `hiking`
- `wheelchair`
- `public-transport`
There are some default profile keys in our standard ors-config.yml with flag encoders and recommended profile-specific settings predefined.
These standard profiles are:

| `*` in `ors.engine.profiles.*` | flag encoder `ors.engine.profiles.*.profile` |
|--------------------------------|----------------------------------------------|
| `car` | `driving-car` |
| `hgv` | `driving-hgv` |
| `bike-regular` | `cycling-regular` |
| `bike-mountain` | `cycling-mountain` |
| `bike-road` | `cycling-road` |
| `bike-electric` | `cycling-electric` |
| `walking` | `foot-car` |
| `hiking` | `foot-hgv` |
| `wheelchair` | `wheelchair` |
| `public-transport` | `public-transport` |

::: warning
If you specified `profile_default` settings they might not be taken into account!
This will be fixed in the next patch release.
As a workaround, you can move all `profile_default` settings to the specific profile where you need them to work.
The predefined settings override settings specified in the `profile_default` in your ors-config.yml or ors-config.env!
If you want to specify your own profile settings based on your specific `profile_default` values, you can work around this by naming your profile differently, e.g. `custom-car` instead of `car`.
Note that the profile name can be chosen freely but cannot contain special characters that cannot be used for directory names on your operating system.
:::

::: warning
In the directions endpoint, the profiles are addressed by their encoder name (e.g. `driving-car`)!
:::

Properties for each (enabled) profile are set under `ors.engine.profiles.<profile>`, e.g.
Expand All @@ -27,7 +35,7 @@ Properties for each (enabled) profile are set under `ors.engine.profiles.<profil

| key | type | description | default value |
|-------------------------------------|---------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|
| profile | string | Profile name | _NA_ |
| profile | string | Profile name for the directions endpoint. Also used as specification for the flag encoder during graph building. Possible values are restricted to those in the second column in above table! | _NA_ |
| enabled | boolean | Enables or disables the profile across **openrouteservice** endpoints | `true` |
| elevation | boolean | Specifies whether to use or not elevation data | `false` |
| elevation_smoothing | boolean | Smooth out elevation data | `false` |
Expand All @@ -36,7 +44,7 @@ Properties for each (enabled) profile are set under `ors.engine.profiles.<profil
| instructions | boolean | Specifies whether way names will be stored during the import or not | `true` |
| optimize | boolean | Optimize the sort order when contracting nodes for CH. This is rather expensive, but yields a better contraction hierarchy. | `false` |
| graph_path | string | Subdirectory name under `ors.engine.graphs_root_path`. If left unset, the profile entry name on the `profiles` list is used | _NA_ |
| encoder_options | string | For details see [encoder_options](#encoder-options) below | |
| encoder_options | string | For details see [encoder_options](#encoder_options) below | |
| preparation | object | [Preparation settings](#preparation) for building the routing graphs | |
| execution | object | [Execution settings](#execution) relevant when querying services | |
| ext_storages | object | [External storages](#ext_storages) for returning extra information | |
Expand Down
Loading

0 comments on commit ae528d3

Please sign in to comment.