Skip to content

Allow custom minSize, maxSize and defaultSize values#1688

Merged
missinglink merged 8 commits intopelias:masterfrom
arnesetzer:master
Mar 7, 2025
Merged

Allow custom minSize, maxSize and defaultSize values#1688
missinglink merged 8 commits intopelias:masterfrom
arnesetzer:master

Conversation

@arnesetzer
Copy link
Contributor

@arnesetzer arnesetzer commented Mar 6, 2025

👋 I did some awesome work for the Pelias project and would love for everyone to have a look at it and provide feedback.


Here's the reason for this change 🚀

Closes #1649


Here's what actually got changed 👏

Allows users to modify their max/max/defaultSize in the pelias.json. If no values are given it will defaults to the original ones (1,40,10) so this PR should be backwards compatible.


Here's how others can test the changes 👀

Spin up an instance, set some custom values in the pelias.json
(eg:

"api": {
    "services": {
      "pip": {
        "url": "http://localhost:4200"
      },
      "libpostal": {
        "url": "http://localhost:4400"
      },
      "placeholder": {
        "url": "http://localhost:4100"
      },
      "interpolation": {
        "url": "http://localhost:4300"
      }
    },
    "minSize": 2,
    "maxSize": 5,
    "defaultSize": 5
  },

) and play around with the size parameter (eg. size=6 should throw a warning).
There are also a few tests to verify that everything runs as expected :)

Copy link
Member

@orangejulius orangejulius left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for this. We do get a lot of requests here so I'm sure it will help people out.

I left some fairly minor suggestions for changes but overall it looks good.

@arnesetzer
Copy link
Contributor Author

Thanks for the feedback. Added your requested changes. Hope that met what you had in mind.

@missinglink
Copy link
Member

I can take this over and merge it, I'll push to your branch

@missinglink missinglink force-pushed the master branch 2 times, most recently from a503a66 to 6685f83 Compare March 7, 2025 13:25
@missinglink
Copy link
Member

This is kinda hard to test with our current setup since calling require('sanitizer/_size.js') gets cached, so, short of wrapping it in a setup function purely for testing it's impossible to test both the defaults and the config vars 🤷‍♂️

That said, I confirmed it works as expected by making these edits (which I didn't commit) and running the tests, the failures are expected.

diff --git a/test/test-pelias-config.json b/test/test-pelias-config.json
index e730d72f..d0488295 100644
--- a/test/test-pelias-config.json
+++ b/test/test-pelias-config.json
@@ -2,6 +2,9 @@
   "api": {
     "targets": {
       "auto_discover": false
-    }
+    },
+    "minSize": 10,
+    "maxSize": 20,
+    "defaultSize": 15
   }
 }
  Failed Tests:   There were 11 failures

    x default to 1
    x default to 40
    x default to 10
    x should return warning
    x set to correct integer
    x should return warning
    x set to correct integer
    x should return warning
    x set to correct integer
    x should return warning
    x set to correct integer

@missinglink missinglink merged commit f1e4575 into pelias:master Mar 7, 2025
3 checks passed
@missinglink
Copy link
Member

agh crap, after I merged this I noticed a minor error, probably not a big deal?

diff --git a/schema.js b/schema.js
index 462f6090..7c3be6a7 100644
--- a/schema.js
+++ b/schema.js
@@ -31,7 +31,7 @@ module.exports = Joi.object().keys({
     }),
     minSize: Joi.number().integer().min(1).max(Joi.ref('maxSize')).default(1),
     maxSize: Joi.number().integer().min(1).default(40),
-    defaultSize: Joi.number().min(Joi.ref('minSize')).max(Joi.ref('maxSize')).default(10),
+    defaultSize: Joi.number().integer().min(Joi.ref('minSize')).max(Joi.ref('maxSize')).default(10),
     localization: Joi.object().keys({
       flipNumberAndStreetCountries: Joi.array().items(Joi.string().regex(/^[A-Z]{3}$/))
     }).unknown(false),

@arnesetzer
Copy link
Contributor Author

This allows the user to use floats as max-size elements which can lead to a permanent warning, when size the size parameter is near the maxSize limit (eg. maxSize=40.5 and size=40). But the api still works normally.

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.

Increase MAX_SIZE limit for Pelias API

3 participants