Skip to content

Conversation

@SyntaxColoring
Copy link
Contributor

@SyntaxColoring SyntaxColoring commented Feb 5, 2022

Overview

These commits deselect a bunch of unused packages in an attempt to free up some filesystem space. (See Opentrons/opentrons#8184.)

Unfortunately, I still don't think this is enough for a top-level make push to succeed, so Opentrons/opentrons#8184 will stay open.

Changelog

  • Removed Python packages for JSON-RPC.
    • Per @sfoster:

      it was a first attempt at making a nice network-transparent interface to the hardware controller for test engineering and others who want to directly interact with it

      i think we're moving away from the concept, you can remove it

    • Also removed chardet, a dependency of this.
  • Removed pyserial-asyncio. I couldn't find any instance of import serial_asyncio in Opentrons/opentrons.
  • Removed json-c, a library that isn't mandated by anything else that we have installed.
  • Removed a bunch of nginx modules that appeared unused to me, judging by our nginx.conf.
  • Removed sl. 😢

Review requests

Do any of the removed packages strike you as something that we should definitely not remove?

@SyntaxColoring SyntaxColoring added the robot-svcs Robot Services label Feb 5, 2022
@SyntaxColoring
Copy link
Contributor Author

Ideas for further work:

  • This PR added git, curl, and wget.

    Do we really need git? Do we really need both curl and wget, or can we just pick one?

  • Busybox has its own implementations of curl and/or wget (can't remember which, exactly). So we have two implementations installed. Can we get rid of one? Are we accidentally doing this with any other commands?

  • Should we strip test files from Python packages, sort of automating the workaround in bug: robot's filesystem is out of space,make push sometimes fails opentrons#8184?

Copy link
Member

@sfoster1 sfoster1 left a comment

Choose a reason for hiding this comment

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

This looks good but I think there's more we can do like,

  • Strip out all the bluetooth stuff. this was a temporary experiment that at this point I think is not going to happen
  • if busybox has a curl let's use that; wget can still be useful, but i don't really care whether it happens or not
  • i do not remember at all why we added git, let's not
  • i doubt we need screen or tmux but they're also tiny so whatever

Stripping as much non-critical content from the python stuff is absolutely a good idea. We may also find it useful to install them as gzipped wheels to save disk space.

BR2_PACKAGE_NETWORK_MANAGER=y
BR2_PACKAGE_NGINX=y
BR2_PACKAGE_NGINX_HTTP_SSL_MODULE=y
# BR2_PACKAGE_NGINX_HTTP_CHARSET_MODULE is not set
Copy link
Member

Choose a reason for hiding this comment

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

huh, stuff like this usually doesn't make it into a defconfig, wonder if nginx is doing something weird to parse these. would expect them to be BR2_PACKAGE_NGINX_HTTP_GZIP_MODULE=n and so on

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it is weird and mysterious.

@SyntaxColoring SyntaxColoring merged commit e9054c3 into opentrons-develop Feb 7, 2022
@SyntaxColoring SyntaxColoring deleted the fs_space branch February 7, 2022 15:22
@SyntaxColoring
Copy link
Contributor Author

Aggregating those further ideas in #144.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

robot-svcs Robot Services

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants