Adding conda recipe #176
Adding conda recipe #176
Conversation
|
@microsoft-github-policy-service agree |
…lines that were conda-forge specific
…isted and added v prefix
|
I tested installing the conda recipe locally and was able to use playwright-pytest. I found the conda-recipe PR 657 for playwright-python so I'll try updating the CI config accordingly although I don't understand how some of it like the conda_build_config.yaml is used. |
| - pip | ||
| run: | ||
| - python >=3.8 | ||
| - playwright >=v1.33.0 |
There was a problem hiding this comment.
The way playwright-python (and now playwright-pytest) create their conda versions from the v prefixed version git tags makes this look a little funny (and tripped me up for a while in testing) as it isn't very consistent with the pure semantic version number or date version number most other conda packages use. Not sure whether you might want to exclude that v prefix at some point.
There was a problem hiding this comment.
Good finding! Lets remove the v prefix directly for the pytest plugin and we can follow-up with a fix for the normal playwright plugin. Do you think this is a breaking change? SemVer should behave similar in both cases I feel like.
|
This is ready for another review when you have time. To paraphrase, I've tested the meta.yaml recipe and installed it fine locally and run a playwright pytest using one of the playwright-pytest arguments with it. I haven't tested the CI config - which I copied almost verbatim from playwright-python - other than adding the microsoft and conda-forge channel arguments to the conda build call. |
Co-authored-by: Max Schmitt <max@schmitt.mx>
Co-authored-by: Max Schmitt <max@schmitt.mx>
|
Thanks for the feedback @mxschmitt - I'm not sure about this timeout CI failure... I don't think it is a consequence of the changes - I'm guessing it is some intermittent problem. I don't have permission to rerun the job and I didn't want to push a dummy commit to force it. Did you want to rerun and check if it is just temporary? |
|
Thanks for finishing this up @mxschmitt ! I hope the CD works okay... will do what I can to help if it has any issues. |
|
@ownenlamont its failing for some reason, do you have a clue why? |
|
Damn... I'll have a look now |
|
You seem to be on the right track, something about the conda build arguments seems wrong. I was guessing how to specify the channels when including them in the CI script since it needs to check both the Microsoft and conda-forge channels. I'll try looking at the conda build code some more to see if I can make sense of it because nothing jumps out at me as wrong yet... 😞 |
|
Did you commit 65eb297 fix the problem @mxschmitt ? I know when I built the package locally I needed to explicitly pull in the Microsoft and conda-forge channels but not sure how that works in the CI environment. |
|
I'm looking for other examples of repos that build conda packages with multiple channel dependencies - I did find one of the huggingface repos that does this here. I still can't make much sense of that error message but I think the microsoft and conda-forge channels should be added to CI in the same way that huggingface repo does it by comma-delimiting the required channels in the Get Conda step:
|
|
The other possibility is conda-build itself is broken, that file throwing the error was touched a few times in the last release of conda-build. I didn't see any open issues on conda-build that exactly matched that error, but there were other issues that appeared to be parsing problems that throw attribute errors. I'm not sure how to pin down the version of conda-build though to test that theory. |
|
Sorry I might have to leave it there for today @mxschmitt - I think you will need to specify the microsoft channel as I mentioned but that doesn't appear to be the cause of your immediate error (I would expect it to have some error about unresolvable dependencies if it got that far). I am suspicious of this latest version (3.26.0) of conda build itself but not sure how to try a lower version to test if that is the problem (I was using version 3.25.0 when I built the package locally on my laptop). |
|
Thanks! I had to massage it a bit, see the history. It's live now: https://anaconda.org/Microsoft/pytest-playwright |
|
Wow, that was a few hoops to jump through. Thanks for persisting with it, I really appreciate it! |
|
Ugh... that's embarrassing, I just realised I reversed the package name in the conda recipe calling it pytest-playwright instead of playwright-pytest (I think having python-playwright gets me mixed up as to whether playwright is the suffix or prefix) and I put in the wrong way round in the meta.yaml. Should we switch it the right way around @mxschmitt? I'm sorry for that silly blunder. I don't know how much hassle it is to take down and put up a package again. |
|
That's the correct naming, otherwise pytest won't discover it, all good! It needs the pytest- prefix. |
|
Oh okay, that's a relief. I spotted the inconsistency when raising that PR for Panel and thought I'd totally messed up. |
Attempting to address issue 113 - I created a recipe using Grayskull and tweaked it to more closely match the format of the playwright-python meta.yml.
I have successfully installed the the recipe locally to a conda environment from the meta.yaml recipe and I've copied the CI config I found the adding the conda recipe in PR 657 for playwright-python. I haven't (and don't know if I could even) test the pipeline steps.