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 uploading a language pack #3815

Merged
merged 1 commit into from
Nov 16, 2020
Merged

Fix uploading a language pack #3815

merged 1 commit into from
Nov 16, 2020

Conversation

cp2004
Copy link
Member

@cp2004 cp2004 commented Nov 14, 2020

In Python 3, filter() returns an iterable rather than a list. Whilst this is better for performance, our list is not long and it means we can call len() on the list.

Tested using reproduction steps in #3814 and verified that language packs can now be installed properly.

Closes #3814

  • Your changes are not possible to do through a plugin and relevant
    to a large audience (ideally all users of OctoPrint)
  • If your changes are large or otherwise disruptive: You have
    made sure your changes don't interfere with current development by
    talking it through with the maintainers, e.g. through a
    Brainstorming ticket
  • Your PR targets OctoPrint's devel branch if it's a completely
    new feature, or maintenance if it's a bug fix or improvement of
    existing functionality for the current stable version (no PRs
    against master or anything else please)
  • Your PR was opened from a custom branch on your repository
    (no PRs from your version of master, maintenance or devel please),
    e.g. dev/my_new_feature or fix/my_bugfix
  • Your PR only contains relevant changes: no unrelated files,
    no dead code, ideally only one commit - rebase and squash your PR
    if necessary!
  • Your changes follow the existing coding style
  • If your changes include style sheets: You have modified the
    .less source files, not the .css files (those are generated with
    lessc)
  • You have tested your changes (please state how!) - ideally you
    have added unit tests
  • You have run the existing unit tests against your changes and
    nothing broke
  • You have added yourself to the AUTHORS.md file :)

What does this PR do and why is it necessary?

Fixes issue uploading language packs on Python 3. This doesn't appear to be a regression in 1.5.0rc1, and a change introduced by 63723ba that removed a change introduced by 30359f5, both claiming to do the same thing.

How was it tested? How can it be tested by the reviewer?

Follow steps outlined in #3814

Any background context you want to provide?

#3814 and #3802 (comment)

What are the relevant tickets if any?

#3814 and #3802 (comment)

Screenshots (if appropriate)

Further notes

Not sure if this counts as a regression, since it has been an issue since 1.4.0 was released, and only when using Python 3. However, since more and more users are moving to Python 3, or installing by default now (both manual and soon OctoPi) I think this should be included in 1.5.0. But at the end of the day, its not up to me. Change the branch to whatever you think works for this fix 🙂

@cp2004 cp2004 changed the title 🐛 Fix uploading a language pack Fix uploading a language pack Nov 14, 2020
In Python 3, `filter()` returns an iterable rather than a list. Whilst this is better for performance, our list is not long and it means we can call `len()` on the list.

Tested using reproduction steps in #3814 and verified that language packs can now be installed properly.

Closes #3814
@foosel foosel added this to the 1.5.0 milestone Nov 16, 2020
@foosel foosel merged commit d311fb1 into OctoPrint:staging/maintenance Nov 16, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants