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

fix: call 'split' on string-type object, not on version-type object #8037

Merged
merged 2 commits into from
Sep 14, 2023
Merged

fix: call 'split' on string-type object, not on version-type object #8037

merged 2 commits into from
Sep 14, 2023

Conversation

fredrikaverpil
Copy link
Contributor

@fredrikaverpil fredrikaverpil commented Sep 14, 2023

This hopefully fixes #8003 which exhibits the following error:

updater | 2023/09/14 09:33:53 ERROR <job_721897648> Error processing pytz (NoMethodError)
updater | 2023/09/14 09:33:53 ERROR <job_721897648> undefined method `split' for #<Dependabot::Python::Version 2023.3.post1>
updater | 
updater |         version, @local_version = version.split("+")
updater |                                          ^^^^^^

The error message is indicating that the split method is being called on an instance of Dependabot::Python::Version, not a string. The split method is a string method, so it cannot be called on an object of a different type unless it implements its own "split" method, which this "Version" type doesn't seem to do.

cc @carogalvin 👋 😅 sorry for pinging you here but this seems to be a showstopper for grouped dependabot PRs with Python at the moment. I started seeing this error as I added grouped dependencies like so:

version: 2

updates:
  - package-ecosystem: "github-actions"
    directory: "/"
    registries: "*"
    schedule:
      interval: "weekly"
      day: "tuesday"
  - package-ecosystem: "pip"
+     groups:
+       production-dependencies:
+         dependency-type: "production"
+         update-types:
+           - minor
+           - patch
+       development-dependencies:
+         dependency-type: "development"
+         update-types:
+           - minor
+           - patch
    insecure-external-code-execution: allow
    directory: "/"
    registries: "*"
    schedule:
      interval: "weekly"
      day: "tuesday"
    ignore:
      - dependency-name: "dr"
    open-pull-requests-limit: 50
    allow:
      - dependency-name: "*"
        dependency-type: all
    commit-message:
      prefix: "chore"
      include: "scope"
    versioning-strategy: increase-if-necessary

registries:
  github:
    type: git
    url: https://github.com/
    username: x-access-token
    password: ${{secrets.REDACTED_TOKEN}}

  cloudsmith:
    type: python-index
    url: https://dl.cloudsmith.io/<REDACTED>/<ORG>/<REPO>/python/simple/
    replaces-base: true

I'm no expert in ruby, so take this PR as just an indication as to what might be wrong 😅

@fredrikaverpil fredrikaverpil requested a review from a team as a code owner September 14, 2023 11:26
@Nishnha Nishnha added the F: grouped-updates 🎳 Relates to bumping more than one dependency in a single PR label Sep 14, 2023
Copy link
Member

@jakecoffman jakecoffman left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! I found an open source repo with the same issue and verified this does fix it. I'll deploy this shortly.

@jakecoffman jakecoffman merged commit 6e1996c into dependabot:main Sep 14, 2023
80 checks passed
@@ -29,7 +29,7 @@ def self.correct?(version)

def initialize(version)
@version_string = version.to_s
version, @local_version = version.split("+")
Copy link
Contributor

Choose a reason for hiding this comment

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

We should add a test for this in a follow up PR 🙏

Copy link
Member

Choose a reason for hiding this comment

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

@yeikel if you're up for it, I'd be more than happy to review and merge it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F: grouped-updates 🎳 Relates to bumping more than one dependency in a single PR L: python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error processing <package> (NoMethodError): undefined method `split'
5 participants