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

[py] moved MANIFEST.in contents to pyproject.toml #14680

Merged
merged 5 commits into from
Nov 9, 2024

Conversation

sandeepsuryaprasad
Copy link
Contributor

@sandeepsuryaprasad sandeepsuryaprasad commented Oct 30, 2024

User description

Thanks for contributing to Selenium!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

In this PR I have moved all the information about the non-default/data files that gets shipped as part of selenium package from MANIFEST.in to pyproject.toml file.

Motivation and Context

This eliminates the need of having a separate config file MANIFEST.in to include all data/non-default files in package.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

enhancement, configuration changes


Description

  • Moved the contents of MANIFEST.in to pyproject.toml, streamlining the configuration by using a single file.
  • Removed MANIFEST.in from the project, as its contents are now managed within pyproject.toml.
  • Updated pyproject.toml to include package data configuration, specifying which files should be included in the package.
  • Adjusted the Bazel build configuration to reflect the removal of MANIFEST.in.

PRDescriptionHeader.CHANGES_WALKTHROUGH

Relevant files
Configuration changes
BUILD.bazel
Remove MANIFEST.in from Bazel build configuration               

py/BUILD.bazel

  • Removed MANIFEST.in from the list of source files for the
    selenium-sdist-pkg.
  • +0/-1     
    MANIFEST.in
    Remove MANIFEST.in file                                                                   

    py/MANIFEST.in

    • Deleted the entire MANIFEST.in file.
    +0/-24   
    pyproject.toml
    Configure package data in pyproject.toml                                 

    py/pyproject.toml

  • Added package data configuration to include necessary files in the
    package.
  • Defined package inclusion and exclusion rules.
  • +22/-0   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Configuration Validation
    Ensure that all necessary files are included in the package-data section and that the configuration is correct for the project's structure.

    Copy link
    Contributor

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Specify version requirements for build system dependencies to ensure compatibility and reproducibility

    Consider adding a version specifier to the build system requirements to ensure
    compatibility and reproducibility. This helps prevent potential issues with future
    incompatible versions of setuptools or setuptools-rust.

    py/pyproject.toml [1-3]

     [build-system]
    -requires = ["setuptools", "setuptools-rust"]
    +requires = ["setuptools>=61.0.0", "setuptools-rust>=1.0.0"]
     build-backend = "setuptools.build_meta"
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding version specifiers to the build system requirements is a best practice that ensures compatibility and reproducibility by preventing issues with future incompatible versions. This is a significant improvement for maintaining a stable build environment.

    8
    Use more specific file patterns in package-data to ensure inclusion of all necessary files while avoiding unnecessary ones

    Consider using more specific file patterns in the package-data section to avoid
    including unnecessary files. For example, instead of "*.py", you could list specific
    Python files or use patterns like "
    /*.py" to include all Python files in
    subdirectories.**

    py/pyproject.toml [11-25]

     [tool.setuptools.package-data]
     selenium_package = [
    -    "*.py", 
    -    "*.rst",
    -    "*.json", 
    -    "*.xpi", 
    -    "*.js",
    +    "**/*.py",
    +    "**/*.rst",
    +    "**/*.json",
    +    "**/*.xpi",
    +    "**/*.js",
         "py.typed",
         "prune*",
         "selenium.egg-info*",
    -    "selenium-manager", 
    +    "selenium-manager",
         "selenium-manager.exe",
         "CHANGES",
         "LICENSE"
     ]
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: The suggestion to use more specific file patterns like "**/*.py" can help in ensuring that all necessary files are included, especially those in subdirectories, while avoiding unnecessary files. This enhances the maintainability and accuracy of the package configuration.

    6

    💡 Need additional feedback ? start a PR chat

    @sandeepsuryaprasad
    Copy link
    Contributor Author

    @VietND96 I have re-raised this PR once again by fixing a bug. Can you please re-trigger the workflow for this PR.

    @VietND96 VietND96 added the C-py label Nov 1, 2024
    @sandeepsuryaprasad
    Copy link
    Contributor Author

    @VietND96 would you please consider merging this PR. All the tests are getting are passed.

    @diemol
    Copy link
    Member

    diemol commented Nov 5, 2024

    Maybe @AutomatedTester can have a look first?

    Copy link

    codecov bot commented Nov 5, 2024

    Codecov Report

    All modified and coverable lines are covered by tests ✅

    Project coverage is 59.23%. Comparing base (57f8398) to head (8105e74).
    Report is 894 commits behind head on trunk.

    Additional details and impacted files
    @@            Coverage Diff             @@
    ##            trunk   #14680      +/-   ##
    ==========================================
    + Coverage   58.48%   59.23%   +0.75%     
    ==========================================
      Files          86       91       +5     
      Lines        5270     5856     +586     
      Branches      220      260      +40     
    ==========================================
    + Hits         3082     3469     +387     
    - Misses       1968     2127     +159     
    - Partials      220      260      +40     

    ☔ View full report in Codecov by Sentry.
    📢 Have feedback on the report? Share it here.

    @AutomatedTester
    Copy link
    Member

    can you run bazel build //py:selenium-wheel and install the wheel to make sure that everything is still working after this change?

    @sandeepsuryaprasad
    Copy link
    Contributor Author

    sandeepsuryaprasad commented Nov 5, 2024

    @AutomatedTester Here is the result.
    Screenshot 2024-11-05 at 6 32 34 PM

    Screenshot 2024-11-05 at 6 50 05 PM

    @AutomatedTester
    Copy link
    Member

    and install the wheel to make sure that everything is still working

    Did you do this part? Just want to make sure that Bazel is building the correct wheel for us to upload to pypi

    @sandeepsuryaprasad
    Copy link
    Contributor Author

    sandeepsuryaprasad commented Nov 5, 2024

    and install the wheel to make sure that everything is still working

    Did you do this part? Just want to make sure that Bazel is building the correct wheel for us to upload to pypi

    @AutomatedTester

    ~$ pip3 install selenium-4.26.0.dev202409202351-py3-none-any.whl
    Processing ./selenium-4.26.0.dev202409202351-py3-none-any.whl
    Requirement already satisfied: urllib3<3,>=1.26 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from urllib3[socks]<3,>=1.26->selenium==4.26.0.dev202409202351) (1.26.18)
    Requirement already satisfied: trio~=0.17 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from selenium==4.26.0.dev202409202351) (0.22.0)
    Requirement already satisfied: trio-websocket~=0.9 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from selenium==4.26.0.dev202409202351) (0.9.2)
    Requirement already satisfied: certifi>=2021.10.8 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from selenium==4.26.0.dev202409202351) (2023.7.22)
    Requirement already satisfied: typing_extensions~=4.9 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from selenium==4.26.0.dev202409202351) (4.11.0)
    Requirement already satisfied: websocket-client~=1.8 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from selenium==4.26.0.dev202409202351) (1.8.0)
    Requirement already satisfied: attrs>=19.2.0 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from trio~=0.17->selenium==4.26.0.dev202409202351) (21.4.0)
    Requirement already satisfied: sortedcontainers in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from trio~=0.17->selenium==4.26.0.dev202409202351) (2.4.0)
    Requirement already satisfied: async-generator>=1.9 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from trio~=0.17->selenium==4.26.0.dev202409202351) (1.10)
    Requirement already satisfied: idna in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from trio~=0.17->selenium==4.26.0.dev202409202351) (3.3)
    Requirement already satisfied: outcome in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from trio~=0.17->selenium==4.26.0.dev202409202351) (1.1.0)
    Requirement already satisfied: sniffio in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from trio~=0.17->selenium==4.26.0.dev202409202351) (1.2.0)
    Requirement already satisfied: wsproto>=0.14 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from trio-websocket~=0.9->selenium==4.26.0.dev202409202351) (1.1.0)
    Requirement already satisfied: PySocks!=1.5.7,<2.0,>=1.5.6 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from urllib3[socks]<3,>=1.26->selenium==4.26.0.dev202409202351) (1.7.1)
    Requirement already satisfied: h11<1,>=0.9.0 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from wsproto>=0.14->trio-websocket~=0.9->selenium==4.26.0.dev202409202351) (0.13.0)
    Installing collected packages: selenium
      Attempting uninstall: selenium
        Found existing installation: selenium 4.24.0
        Uninstalling selenium-4.24.0:
          Successfully uninstalled selenium-4.24.0
    Successfully installed selenium-4.26.0.dev202409202351
    
    [notice] A new release of pip is available: 24.2 -> 24.3.1
    [notice] To update, run: pip install --upgrade pip
    
    ~$ pip3 show selenium
    Name: selenium
    Version: 4.26.0.dev202409202351
    Summary: Official Python bindings for Selenium WebDriver
    Home-page: https://www.selenium.dev
    Author:
    Author-email:
    License: Apache 2.0
    Location: /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages
    Requires: certifi, trio, trio-websocket, typing_extensions, urllib3, websocket-client
    Required-by:
    

    @VietND96
    Copy link
    Member

    VietND96 commented Nov 5, 2024

    Via CI, I triggered some validation for your reference.

    You can inspect further in the uploaded whl and source to see all are fine

    @sandeepsuryaprasad
    Copy link
    Contributor Author

    Via CI, I triggered some validation for your reference.

    You can inspect further in the uploaded whl and source to see all are fine

    Sure. I will go through the uploaded files , verify it and will confirm the same.

    @sandeepsuryaprasad
    Copy link
    Contributor Author

    sandeepsuryaprasad commented Nov 6, 2024

    @VietND96 , @AutomatedTester
    I have verified the source that is uploaded in PyPI, It looks fine, but in selenium/webdriver/common I can see devtools folder included as well. Just want to confirm whether or not this folder should be included in final source distribution (with current settings in pyproject.toml, this directory would be included as it is a package).

    I installed selenium from the uploaded test PyPI. Below are the results.

    ~$ pip3 install -i https://test.pypi.org/simple/ selenium==4.27.0.dev202411051921
    Looking in indexes: https://test.pypi.org/simple/
    Collecting selenium==4.27.0.dev202411051921
      Downloading https://test-files.pythonhosted.org/packages/3c/c4/59081abb32dd133f1f5720f9fd78c6d27ea2a120de04e1c4db8ed82e4c94/selenium-4.27.0.dev202411051921-py3-none-any.whl.metadata (7.1 kB)
    Requirement already satisfied: urllib3<3,>=1.26 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from urllib3[socks]<3,>=1.26->selenium==4.27.0.dev202411051921) (1.26.18)
    Requirement already satisfied: trio~=0.17 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from selenium==4.27.0.dev202411051921) (0.22.0)
    Requirement already satisfied: trio-websocket~=0.9 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from selenium==4.27.0.dev202411051921) (0.9.2)
    Requirement already satisfied: certifi>=2021.10.8 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages (from selenium==4.27.0.dev202411051921) (2023.7.22)
    Requirement already satisfied: typing_extensions~=4.9 in /Library/Frameworks/Python.framework/Versions/3.11/lib/pyt3.11/site-packages (from selenium==4.27.0.dev202411051921) (0.9.2)                               08:38:01 [44/1831]
    Requirement already satisfied: certifi>=2021.10.8 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3
    .11/site-packages (from selenium==4.27.0.dev202411051921) (2023.7.22)
    Requirement already satisfied: typing_extensions~=4.9 in /Library/Frameworks/Python.framework/Versions/3.11/lib/pyt
    hon3.11/site-packages (from selenium==4.27.0.dev202411051921) (4.11.0)
    Requirement already satisfied: websocket-client~=1.8 in /Library/Frameworks/Python.framework/Versions/3.11/lib/pyth
    on3.11/site-packages (from selenium==4.27.0.dev202411051921) (1.8.0)
    Requirement already satisfied: attrs>=19.2.0 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/s
    ite-packages (from trio~=0.17->selenium==4.27.0.dev202411051921) (21.4.0)
    Requirement already satisfied: sortedcontainers in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.1
    1/site-packages (from trio~=0.17->selenium==4.27.0.dev202411051921) (2.4.0)
    Requirement already satisfied: async-generator>=1.9 in /Library/Frameworks/Python.framework/Versions/3.11/lib/pytho
    n3.11/site-packages (from trio~=0.17->selenium==4.27.0.dev202411051921) (1.10)
    Requirement already satisfied: idna in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packa
    ges (from trio~=0.17->selenium==4.27.0.dev202411051921) (3.3)
    Requirement already satisfied: outcome in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-pa
    ckages (from trio~=0.17->selenium==4.27.0.dev202411051921) (1.1.0)
    Requirement already satisfied: sniffio in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-pa
    ckages (from trio~=0.17->selenium==4.27.0.dev202411051921) (1.2.0)
    Requirement already satisfied: wsproto>=0.14 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/s
    ite-packages (from trio-websocket~=0.9->selenium==4.27.0.dev202411051921) (1.1.0)
    Requirement already satisfied: PySocks!=1.5.7,<2.0,>=1.5.6 in /Library/Frameworks/Python.framework/Versions/3.11/li
    b/python3.11/site-packages (from urllib3[socks]<3,>=1.26->selenium==4.27.0.dev202411051921) (1.7.1)
    Requirement already satisfied: h11<1,>=0.9.0 in /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/s
    ite-packages (from wsproto>=0.14->trio-websocket~=0.9->selenium==4.27.0.dev202411051921) (0.13.0)
    Downloading https://test-files.pythonhosted.org/packages/3c/c4/59081abb32dd133f1f5720f9fd78c6d27ea2a120de04e1c4db8e
    d82e4c94/selenium-4.27.0.dev202411051921-py3-none-any.whl (9.7 MB)
       ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 9.7/9.7 MB 3.8 MB/s eta 0:00:00
    Installing collected packages: selenium
      Attempting uninstall: selenium
        Found existing installation: selenium 4.26.0.dev202409202351
        Uninstalling selenium-4.26.0.dev202409202351:
          Successfully uninstalled selenium-4.26.0.dev202409202351
    Successfully installed selenium-4.27.0.dev202411051921
    

    @sandeepsuryaprasad
    Copy link
    Contributor Author

    @VietND96, @AutomatedTester, can we have this one merged?

    @AutomatedTester
    Copy link
    Member

    Via CI, I triggered some validation for your reference.

    * The change is built Nightly and uploaded to TestPypi: https://test.pypi.org/project/selenium/4.27.0.dev202411051921/#files
    
    * A set of example tests in docs repo run on top of that Nightly build - https://github.com/SeleniumHQ/seleniumhq.github.io/actions/runs/11691201375
    

    You can inspect further in the uploaded whl and source to see all are fine

    This is what I was asking for repeatedly. This looks great!

    @AutomatedTester AutomatedTester merged commit 3e1cb0c into SeleniumHQ:trunk Nov 9, 2024
    36 of 38 checks passed
    @sandeepsuryaprasad sandeepsuryaprasad deleted the mfest branch November 24, 2024 04:14
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    4 participants