Skip to content

fix: evaluate profile_default #1767

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

Merged
merged 27 commits into from
Apr 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
74230bc
fix: move yml properties to defaults where possible
jhaeu Mar 27, 2024
80aef17
fix: fix evaluation of 'enabled' in profile_default
jhaeu Mar 27, 2024
aa47d7a
feat: adapt conversion to changed application.yml
jhaeu Mar 27, 2024
a4d0742
feat: add config tests for profile_default
jhaeu Mar 27, 2024
b80901a
fix: do not indent commented lines
jhaeu Mar 27, 2024
f6cdacc
feat: re-generated ors-config.yml
jhaeu Mar 27, 2024
10d77f9
test: add test
jhaeu Apr 18, 2024
4fbd1c4
test: add support for skipping tests
jhaeu Apr 18, 2024
a298359
test: don't use port 8082
jhaeu Apr 18, 2024
b875ce2
test: fix test cleanup
jhaeu Apr 18, 2024
00333b1
docs: explain profile keys vs profile names
jhaeu Apr 18, 2024
1c57d2a
test: separated yml from json in matrix
jhaeu Apr 18, 2024
e8c6d41
docs: update changelog
jhaeu Apr 18, 2024
e022138
docs: fix typo
jhaeu Apr 18, 2024
e6ea33e
docs: do not promise too much
jhaeu Apr 18, 2024
c06fefe
fix: add car enabled to ors-config
jhaeu Apr 19, 2024
a4f3415
fix: fix sed
jhaeu Apr 19, 2024
3b31b82
docs: update profiles.md
takb Apr 19, 2024
13962c8
fix: config conversion failing due to missing enabled property
takb Apr 19, 2024
700cae5
chore(config): automatic conversion of application.yml to ors-config.…
takb Apr 19, 2024
61ee483
fix: uncomment correctly in ors-config.yml
takb Apr 19, 2024
dbf654e
chore(config): automatic conversion of application.yml to ors-config.…
takb Apr 19, 2024
e5bb339
fix: uncomment correctly in ors-config.yml
takb Apr 19, 2024
94a2d07
chore(config): automatic conversion of application.yml to ors-config.…
takb Apr 19, 2024
7ba9652
ci: config update by script requires additional commit
takb Apr 19, 2024
65a866f
test: test enabled false in profile_default
jhaeu Apr 19, 2024
e63588e
chore: add test description, remove comments
jhaeu Apr 19, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading