Fix master #1682

samuelcolvin · 2020-07-04T10:51:51Z

Trying to fix master.

codecov · 2020-07-04T10:54:49Z

Codecov Report

Merging #1682 into master will increase coverage by 0.25%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1682      +/-   ##
==========================================
+ Coverage   99.68%   99.94%   +0.25%     
==========================================
  Files          21       21              
  Lines        3863     3863              
  Branches      767      767              
==========================================
+ Hits         3851     3861      +10     
+ Misses         10        1       -9     
+ Partials        2        1       -1

Impacted Files	Coverage Δ
pydantic/fields.py	`100.00% <ø> (ø)`
pydantic/env_settings.py	`100.00% <100.00%> (+2.59%)`	⬆️
pydantic/types.py	`99.57% <0.00%> (ø)`
pydantic/networks.py	`100.00% <0.00%> (+0.77%)`	⬆️
pydantic/utils.py	`100.00% <0.00%> (+0.89%)`	⬆️
pydantic/typing.py	`100.00% <0.00%> (+1.88%)`	⬆️
pydantic/version.py	`100.00% <0.00%> (+11.76%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dac6764...f035da3. Read the comment docs.

Bobronium · 2020-07-04T12:31:48Z

validate_always is getting set to True causing error because of 8aebba2#diff-1ef405a6b5a2fa9f20ea7e1f92e4c38eR510 combined with dca9855#diff-1ef405a6b5a2fa9f20ea7e1f92e4c38eR468

Bobronium · 2020-07-05T10:25:33Z

The strange part is that tests were passing on windows somehow. This potentially means that pydantic could behave differently on this platform

PrettyWood · 2020-07-06T09:54:37Z

@samuelcolvin Do you want some help to investigate this matter?

samuelcolvin · 2020-07-06T10:03:01Z

This is preventing v1.6 from being released.

Any help on what's going wrong would be useful, I'll try and look later today.

Bobronium · 2020-07-06T12:30:32Z

I see this as a plan of investigating and fixing the issue:

~~Find out why tests were broken~~ — done in Fix master #1682 (comment) and Fix master #1682 (comment)
~~Find out why this changes didn't break tests on windows~~ Fix master #1682 (comment)
Restore expected behaviour while maintaining features introduced in check ModelField().validate_always when inheriting #1545 and Add conset #1627
Fix detecting failing running tests on CI on windows

~~Unfortunately, I don't have IDE installed on windows to investigate, if someone could help with that, it would be great.~~

PrettyWood · 2020-07-06T12:32:36Z

So the real problem is that we have a "regression" with #1545 where

def test_conlist():
    class Model(BaseModel):
        v: conlist(int) = 'a'

    m = Model()
    assert m.v == 'a'

used to be valid with 1.5.1 because validate_always would not be set to True but now it does.
Do we actually need the always=True in list validator (same for recently added set) ?

PrettyWood · 2020-07-07T05:21:42Z

Meh I just tried on a windows 10 and python 3.8.2:

tests\test_types.py:249 (test_constrained_set_default)
def test_constrained_set_default():
        class ConSetModelMax(BaseModel):
            v: conset(int) = {}
    
>       m = ConSetModelMax()

Would be nice to be able to connect via SSH on the windows 2019 server machine of github

Bobronium · 2020-07-07T07:47:17Z

Well, I looked at CI output closer and noticed that tests actually failed, but CI did not detect it for some reason:
(and yeah, test_env_file_custom_encoding fails on windows ever since it was introduced)

================================== FAILURES ===================================
________________________ test_env_file_custom_encoding ________________________

tmp_path = WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/test_env_file_custom_encoding0')

    @pytest.mark.skipif(not dotenv, reason='python-dotenv not installed')
    def test_env_file_custom_encoding(tmp_path):
        p = tmp_path / '.env'
        p.write_text('pika=p!�@', encoding='latin-1')
    
        class Settings(BaseSettings):
            pika: str
    
        with pytest.raises(UnicodeDecodeError):
>           Settings(_env_file=str(p))
E           Failed: DID NOT RAISE <class 'UnicodeDecodeError'>

tests\test_settings.py:557: Failed

________________________ test_constrained_set_default _________________________

    def test_constrained_set_default():
        class ConSetModelMax(BaseModel):
            v: conset(int) = {}
    
>       m = ConSetModelMax()

tests\test_types.py:254: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

__pydantic_self__ = ConSetModelMax(), data = {}, values = {}, fields_set = set()
validation_error = ValidationError(model='ConSetModelMax', errors=[{'loc': ('v',), 'msg': 'value is not a valid set', 'type': 'type_error.set'}])

    def __init__(__pydantic_self__, **data: Any) -> None:
        """
        Create a new model by parsing and validating input data from keyword arguments.
    
        Raises ValidationError if the input data cannot be parsed to form a valid model.
        """
        # Uses something other than `self` the first arg to allow "self" as a settable attribute
        if TYPE_CHECKING:
            __pydantic_self__.__dict__: Dict[str, Any] = {}
            __pydantic_self__.__fields_set__: 'SetStr' = set()
        values, fields_set, validation_error = validate_model(__pydantic_self__.__class__, data)
        if validation_error:
>           raise validation_error
E           pydantic.error_wrappers.ValidationError: 1 validation error for ConSetModelMax
E           v
E             value is not a valid set (type=type_error.set)

pydantic\main.py:346: ValidationError

=========================== short test summary info ===========================
FAILED tests/test_settings.py::test_env_file_custom_encoding - Failed: DID NO...
FAILED tests/test_types.py::test_constrained_set_default - pydantic.error_wra...
================= 2 failed, 1707 passed, 9 skipped in 27.38s ==================
mingw32-make: *** [Makefile:42: test] Error 1

PrettyWood · 2020-07-07T11:08:20Z

Thanks for checking @MrMrRobat. It's reassuring! I was afraid it was ok only on some windows systems.
So the next question is "How do we fix the CI on windows to actually fail?"
@samuelcolvin @MrMrRobat #1694 is a proposal to fix tests on windows and avoid potential regressions. There is still the issue of the CI that does not fail on windows even though tests fail

samuelcolvin · 2020-07-07T15:41:01Z

this is really weird, I've done quite a lot of digging but can't find anyone else having this problem with steps passing when make commands fail.

I think it must be some weird way gnu make and powershell are not working correctly together, but I can't find any clues about what's wrong.

PrettyWood · 2020-07-07T17:56:04Z

@samuelcolvin Don't you think we could ask someone from github (actions) to have a look ? Nothing has changed on pydantic side regarding the CI. The exit code is 1 and fails on local. And we are talking about make, which is used like in every CI and has been stable for so long. I don't know if something has changed on github action side though. Maybe a new windows OS or a new binary for gnu make ? I don't think you can connect via SSH on the machine to have a deeper look (like on CircleCI) but if you can maybe it's worth a try too?

Kilo59 · 2020-07-07T18:14:52Z

@samuelcolvin @PrettyWood
Could pydantic move away from make and start using invoke?
Then we don't have to rely on gnu make at all.
http://www.pyinvoke.org/

Some examples from another project I work on.
https://github.com/ExpDev07/coronavirus-tracker-api/blob/master/tasks.py

PrettyWood · 2020-07-07T18:27:57Z

Thanks for sharing @Kilo59 (and kudos to you for this project!). But I don't think the issue is to know if make should be used or not but first to understand why the CI fails. Samuel said it could come from make but nothing is sure! And anyway if there is an issue here, it might well happen on other projects so it could be nice to dig into it and understand it. There are indeed a lot of projects that use make (personally I use it in all my python projects!)

* test: add regression test with wrong type default * fix: remove always on conlist and conset * fix: use utf8 as default encoding on all OS

samuelcolvin · 2020-07-09T13:26:31Z

@MrMrRobat @PrettyWood thank you both for your help. I'm working on the windows problem now on #1700

Fix master

66a4d3b

samuelcolvin mentioned this pull request Jul 4, 2020

Add conset #1627

Merged

4 tasks

samuelcolvin mentioned this pull request Jul 4, 2020

schema ref_template #1480

Merged

5 tasks

Kilo59 approved these changes Jul 4, 2020

View reviewed changes

PrettyWood mentioned this pull request Jul 7, 2020

Fix master 2 #1694

Merged

4 tasks

PrettyWood and others added 3 commits July 9, 2020 13:29

Fix master 2 (#1694)

4658cf8

* test: add regression test with wrong type default * fix: remove always on conlist and conset * fix: use utf8 as default encoding on all OS

conset invalid defualt test

de861ab

add change

f035da3

samuelcolvin merged commit a913c37 into master Jul 9, 2020

samuelcolvin deleted the fix-master branch July 9, 2020 13:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix master #1682

Fix master #1682

samuelcolvin commented Jul 4, 2020

codecov bot commented Jul 4, 2020 •

edited

Bobronium commented Jul 4, 2020 •

edited

Bobronium commented Jul 5, 2020

PrettyWood commented Jul 6, 2020

samuelcolvin commented Jul 6, 2020

Bobronium commented Jul 6, 2020 •

edited

PrettyWood commented Jul 6, 2020 •

edited

PrettyWood commented Jul 7, 2020

Bobronium commented Jul 7, 2020 •

edited

PrettyWood commented Jul 7, 2020 •

edited

samuelcolvin commented Jul 7, 2020

PrettyWood commented Jul 7, 2020

Kilo59 commented Jul 7, 2020 •

edited

PrettyWood commented Jul 7, 2020 •

edited

samuelcolvin commented Jul 9, 2020

Fix master #1682

Fix master #1682

Conversation

samuelcolvin commented Jul 4, 2020

codecov bot commented Jul 4, 2020 • edited

Codecov Report

Bobronium commented Jul 4, 2020 • edited

Bobronium commented Jul 5, 2020

PrettyWood commented Jul 6, 2020

samuelcolvin commented Jul 6, 2020

Bobronium commented Jul 6, 2020 • edited

PrettyWood commented Jul 6, 2020 • edited

PrettyWood commented Jul 7, 2020

Bobronium commented Jul 7, 2020 • edited

PrettyWood commented Jul 7, 2020 • edited

samuelcolvin commented Jul 7, 2020

PrettyWood commented Jul 7, 2020

Kilo59 commented Jul 7, 2020 • edited

PrettyWood commented Jul 7, 2020 • edited

samuelcolvin commented Jul 9, 2020

codecov bot commented Jul 4, 2020 •

edited

Bobronium commented Jul 4, 2020 •

edited

Bobronium commented Jul 6, 2020 •

edited

PrettyWood commented Jul 6, 2020 •

edited

Bobronium commented Jul 7, 2020 •

edited

PrettyWood commented Jul 7, 2020 •

edited

Kilo59 commented Jul 7, 2020 •

edited

PrettyWood commented Jul 7, 2020 •

edited