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

monkey_patches an issue with Pydantic causing PageLinks validation to fail #2298

Merged
merged 18 commits into from
May 1, 2021

Conversation

GitHK
Copy link
Contributor

@GitHK GitHK commented Apr 27, 2021

What do these changes do?

The new dynamic-sidecar requires urls in the following format http://10.0.0.1.nip.io for local development, which were not properly handled by Pydantic's url validator.

There is an issue with the url validation inside Pydantic. Waiting for this PR pydantic/pydantic#2512 to be properly merged and released.

  • added checks to help with the maintenance to remove this mokeypatch.
  • extended test suite to cover this case

Related issue/s

required by #1887

How to test

Checklist

@GitHK GitHK added bug buggy, it does not work as expected a:webserver issue related to the webserver service labels Apr 27, 2021
@GitHK GitHK self-assigned this Apr 27, 2021
@GitHK GitHK added this to the Schwarznasenschaf milestone Apr 27, 2021
@codecov
Copy link

codecov bot commented Apr 27, 2021

Codecov Report

Merging #2298 (eaca86e) into master (e72a398) will increase coverage by 0.0%.
The diff coverage is 81.2%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2298   +/-   ##
======================================
  Coverage    71.7%   71.8%           
======================================
  Files         506     506           
  Lines       19896   19912   +16     
  Branches     1947    1949    +2     
======================================
+ Hits        14283   14298   +15     
  Misses       5140    5140           
- Partials      473     474    +1     
Flag Coverage Δ
integrationtests 62.0% <100.0%> (+<0.1%) ⬆️
unittests 67.1% <81.2%> (-0.1%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ce-library/src/servicelib/rest_pagination_utils.py 92.9% <78.5%> (-3.6%) ⬇️
...erver/src/simcore_service_webserver/application.py 93.2% <100.0%> (+0.1%) ⬆️
.../director/src/simcore_service_director/producer.py 60.6% <0.0%> (-0.3%) ⬇️
...webserver/computation_comp_tasks_listening_task.py 91.3% <0.0%> (+3.2%) ⬆️

@GitHK GitHK marked this pull request as ready for review April 27, 2021 08:05
@GitHK GitHK requested review from sanderegg and pcrespov April 27, 2021 08:05
@pcrespov pcrespov changed the title monkey_patcges an issue with Pydantic causing PageLinks validation to fail monkey_patches an issue with Pydantic causing PageLinks validation to fail Apr 27, 2021
@GitHK
Copy link
Contributor Author

GitHK commented May 1, 2021

The monkey patch must be applied manually inside the webserver or the entire CI tagged with [sys] and [int](except [sys] environment setup) ended up failing with errors which made no sense.

@GitHK GitHK merged commit 0af11b5 into ITISFoundation:master May 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:webserver issue related to the webserver service bug buggy, it does not work as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants