Skip to content

define tests workflow, drop obsolete deps#19

Merged
maxlapshin merged 20 commits intoflussonic:masterfrom
stolen:add-github-ci-workflow
Oct 17, 2025
Merged

define tests workflow, drop obsolete deps#19
maxlapshin merged 20 commits intoflussonic:masterfrom
stolen:add-github-ci-workflow

Conversation

@stolen
Copy link
Copy Markdown
Contributor

@stolen stolen commented Sep 28, 2025

  • reduce dependencies: drop lhttpc, yamerl and jsx (when we have json in OTP)
  • add tests workflow across different OTP releases to ensure PRs don't break things and to be aware of compatibility
image

Danil Zagoskin and others added 15 commits July 16, 2025 16:45
fix a crash when ordinary field behind discriminator has wrong format
openapi_schema: boolean values should not be valid strings
…array

fix format validation of array items
…nator-type

openapi_schema: fix error when passed discriminator has wrong type
…r_msg_in_string_length_restriction_case

openapi_schema: extend error message in string length restriction cases
@stolen stolen force-pushed the add-github-ci-workflow branch from 621ea86 to 2df5e7e Compare September 28, 2025 09:55
openapi_schema: count UTF8 code points when checking string length constraints
openapi_schema: fix crash when validating array/map vs const
Comment thread src/openapi_client.erl Outdated
ContentType = proplists:get_value("Content-Type", RequestHeaders, "text/plain"),
{RequestURL, RequestHeaders, ContentType, RequestBody}
end,
Result = case httpc:request(Method, Request, [{timeout, Timeout}], [{body_format, binary}]) of
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

В этом месте мне не нравится следующее (и это серьезная проблема эрланга): ты хочешь убрать зависимость от внешней либы и оставить stdlib, но по сути объявляешь это место невозможным к подсовыванию другой имплементации.

Мы точно не внесем ограничений этим переходом?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Я не вижу серьёзных отличий httpc от lhttpc, если сравнивать с условным hackney -- по сути, у каждого клиента немного свой API, и под это нужно адаптировать код.

Если есть желание сохранить возможность по-быстрому вставить hackney или вроде того, то можно, например строки 65-77 (в предложенной ревизии) вынести в функцию с предсказуемым поведением.
Будет что-то вроде

make_request(RequestURL, Method, RequestHeaders, RequestBody, Timeout) ->
  {ok, Status, [{HdrName :: lowercase_string(), HdrValue :: string()}], RespBody :: binary()} | {error, any()}

Тогда можно будет простым патчем заменить клиент (сохранив сигнатуру), а при желании -- даже принимать альтернативные make_request в опциях.

И, возможно, стоит тут заодно уточнить роль openapi_client:

  • если это тестовый клиент, то stdlib кажется более предпочтительным, потому что он достаточно хорош для выполнения тестовых запросов, а требований про архитектуре, производительности, etc. примерно нет
  • если это продакшн-компонент, то кому-нибудь будет нужен контроль над транспортом (реализация, опции вроде пула, сертификаты TLS, etc.). Тогда понадобится возможность задать свой make_request через опции.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@maxlapshin bump

Сейчас предлагаю два варианта:

  • вынести поход к HTTP в явную функцию с чёткой сигнатурой (делать её параметризуемой когда возникнет потребность)
  • оставить lhttpc как было и отложить решение до новых вводных

Прямо сейчас lhttpc проблем не создаёт.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@maxlapshin я добавил коммит с тем вариантом параметризации, который предлагаю.
Если ок, то напишу на него тест и документацию.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@stolen stolen force-pushed the add-github-ci-workflow branch from 2df5e7e to a8dc196 Compare October 10, 2025 12:54
@maxlapshin maxlapshin merged commit bfa5545 into flussonic:master Oct 17, 2025
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.

3 participants