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

motionEye v0.43.1 release #2373

Open
MichaIng opened this issue Mar 20, 2022 · 62 comments · Fixed by #2608 or #2675
Open

motionEye v0.43.1 release #2373

MichaIng opened this issue Mar 20, 2022 · 62 comments · Fixed by #2608 or #2675
Labels
Milestone

Comments

@MichaIng
Copy link
Member

Most open pull requests have been merged, or major parts of larger collection PRs. Left are minor or layout/GUI additions, or feature PRs which require a rebase with conflicts resolving, all I'd not see as mandatory for a release.

So the question is how we do it.

  • First of all is there anything, especially bugs/issues, any of you want to have addressed first?
  • Then we should go through all features and test them, at best once on x86_64 and once on a Raspberry Pi, where some special features are available. How shall we do it? Adding a ToDo list here that everyone can tick off?
  • The Wiki needs an update, at least the install instructions. We have a pretty generic method in the README.md now. Not sure whether we should add Fedora/rpm and Arch/pkg Python 3 + development headers instructions to README.md, deprecating these Wiki pages, or updating them instead and linking it from the README.md?
  • The Docker Wiki page needs an update as well. Not sure why it contains the whole Dockerfile instead of just linking the existing one 😄. I don't see much point to list Docker Compose instructions as long as motionEye runs fine in a single container? Finally it doubles with the related README.md, so similarly the question is whether we want to keep both, e.g. having install instructions in the Wiki and only container build instructions in the README? We may also want to add instructions about how to install the container from the GitHub registry, until we have a release at Docker Hub, and keeping it for development releases/testing?
  • Then, once we do not find any blocker issues anymore, a pre-release makes sense. I think such is possible on PyPI as well, isn't it? At least there is a --pre flag. @ccrisan can a project on PyPI be transferred somehow, so that pip install motioneye can be done as before without the need to do keep using your personal PyPI account?

Let me know whether I forgot something and what you think about the above.

@ccrisan
Copy link
Collaborator

ccrisan commented Mar 20, 2022

can a project on PyPI be transferred somehow, so that pip install motioneye can be done as before without the need to do keep using your personal PyPI account?

A PyPI token can be added as an (ecrypted) secret to github's secrets and can be used from CI jobs in GitHub Actions.

Unfortunately there's no "organization" concept on PyPI (at least not one that I can find) so I guess my user is just as good as anyone else's.

I just created the organization-level PYPI_TOKEN secret pointing to a new PyPI token scoped to motioneye. As far as I remember, it can be used in the CI like ${{ secrets.PYPI_TOKEN }}.

@MichaIng
Copy link
Member Author

Many thanks. Indeed, practically it shouldn't make a difference whether we keep using your PyPI account or a new one, safe with an access token.

The Docker Hub download is anyway always done with account prefix, so for this a new account wouldn't be a problem, after install instructions have been updated.

@ultratoto14
Copy link
Contributor

Just a small remark, in dev branch, the Dockerfile has been refactored to use an entrypoint and this entrypoint.sh file is not copied in the container, the container cannot start.

@MichaIng
Copy link
Member Author

MichaIng commented Mar 24, 2022

Many thanks for the hint. I'm no Docker expert (and didn't find the time to test the container yet, to be true), can you open a PR to fix it?

I guess ENTRYPOINT ["/entrypoint.sh"] defines the path within the container, but obviously this doesn't imply copying it from the same dir of the Dockerfile during build?

@ultratoto14
Copy link
Contributor

Yep just adding

  COPY . /tmp/motioneye
+COPY extra/entrypoint.sh /entrypoint.sh

should do the trick.
I did not test yet the docker container, it may need additional changes, will try to find time and if anything else is needed, I'll open a PR.

MichaIng added a commit that referenced this issue Mar 24, 2022
#2373 (comment)

Signed-off-by: MichaIng <micha@dietpi.com>
@MichaIng
Copy link
Member Author

PR up: #2386

@spupuz
Copy link

spupuz commented May 15, 2022

Is version 0.43 usable with docker, does this motioneye will be developed in future?

@MichaIng
Copy link
Member Author

MichaIng commented May 15, 2022

Here is the Docker container: https://github.com/motioneye-project/motioneye/pkgs/container/motioneye
Future development of motionEye is the whole point of the https://github.com/motioneye-project organisation, the planned v0.43.0 release etc 😉.

@spupuz
Copy link

spupuz commented May 15, 2022

Is there a docker compose file o saw that it still refer to ccrisan version

@MichaIng
Copy link
Member Author

That needs to be updated indeed (the wiki in general), however, you should be able to extract the essence from it.

@spupuz
Copy link

spupuz commented May 15, 2022

Sorry not sure how to do it

@MichaIng
Copy link
Member Author

Please test with this change: https://github.com/motioneye-project/motioneye/pull/2482/files

@spupuz
Copy link

spupuz commented May 15, 2022

if i change with ghcr.io/motioneye-project/motioneye:edge the images from cam are not loades, do i have to setup it from scratch or can i maintain ccrisan settings?

@spupuz
Copy link

spupuz commented Jul 3, 2023

hello there with latest update i got errors like these:

  File "/usr/local/lib/python3.9/dist-packages/motioneye/mediafiles.py", line 1030, in get_current_picture
    image.thumbnail((width, height), Image.CUBIC)
AttributeError: module 'PIL.Image' has no attribute 'CUBIC'
   ERROR: 500 GET /picture/1/current/?_=1688373818539&width=0.81&_username=admin&_signature=443e749b9bcdec6243f49323d58cee95669091a6 (172.17.0.1) 0.70ms
   ERROR: module 'PIL.Image' has no attribute 'CUBIC'
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/tornado/web.py", line 1786, in _execute
    result = await result
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 52, in get
    await self.current(camera_id)
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 110, in current
    picture = mediafiles.get_current_picture(
  File "/usr/local/lib/python3.9/dist-packages/motioneye/mediafiles.py", line 1030, in get_current_picture
    image.thumbnail((width, height), Image.CUBIC)
AttributeError: module 'PIL.Image' has no attribute 'CUBIC'
   ERROR: 500 GET /picture/5/current/?_=1688373818539&width=0.81&_username=admin&_signature=2e3b8a0be521b6dcfd14970ad72f59431a371b38 (172.17.0.1) 0.48ms
   ERROR: module 'PIL.Image' has no attribute 'CUBIC'
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/tornado/web.py", line 1786, in _execute
    result = await result
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 52, in get
    await self.current(camera_id)
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 110, in current
    picture = mediafiles.get_current_picture(
  File "/usr/local/lib/python3.9/dist-packages/motioneye/mediafiles.py", line 1030, in get_current_picture
    image.thumbnail((width, height), Image.CUBIC)
AttributeError: module 'PIL.Image' has no attribute 'CUBIC'
   ERROR: 500 GET /picture/6/current/?_=1688373818539&width=0.81&_username=admin&_signature=f292d42212edd27766e2efb114caac9a6735ac0e (172.17.0.1) 0.41ms
   ERROR: module 'PIL.Image' has no attribute 'CUBIC'
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/tornado/web.py", line 1786, in _execute
    result = await result
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 52, in get
    await self.current(camera_id)
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 110, in current
    picture = mediafiles.get_current_picture(
  File "/usr/local/lib/python3.9/dist-packages/motioneye/mediafiles.py", line 1030, in get_current_picture
    image.thumbnail((width, height), Image.CUBIC)
AttributeError: module 'PIL.Image' has no attribute 'CUBIC'
   ERROR: 500 GET /picture/3/current/?_=1688373818574&width=0.81&_username=admin&_signature=ce9cd1cd8c1c891ac9d2fa3f0ea9e75606517b0a (172.17.0.1) 0.64ms
   ERROR: module 'PIL.Image' has no attribute 'CUBIC'
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/tornado/web.py", line 1786, in _execute
    result = await result
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 52, in get
    await self.current(camera_id)
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 110, in current
    picture = mediafiles.get_current_picture(
  File "/usr/local/lib/python3.9/dist-packages/motioneye/mediafiles.py", line 1030, in get_current_picture
    image.thumbnail((width, height), Image.CUBIC)
AttributeError: module 'PIL.Image' has no attribute 'CUBIC'
   ERROR: 500 GET /picture/7/current/?_=1688373818574&width=0.81&_username=admin&_signature=d95829b4a734f256ff59510a1aea672e0dc5a9a8 (172.17.0.1) 0.52ms
   ERROR: module 'PIL.Image' has no attribute 'CUBIC'
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/tornado/web.py", line 1786, in _execute
    result = await result
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 52, in get
    await self.current(camera_id)
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 110, in current
    picture = mediafiles.get_current_picture(
  File "/usr/local/lib/python3.9/dist-packages/motioneye/mediafiles.py", line 1030, in get_current_picture
    image.thumbnail((width, height), Image.CUBIC)
AttributeError: module 'PIL.Image' has no attribute 'CUBIC'
   ERROR: 500 GET /picture/10/current/?_=1688373818574&width=0.81&_username=admin&_signature=defdba7286bf4396042eacd8836b26fd99b2f95e (172.17.0.1) 0.41ms
   ERROR: module 'PIL.Image' has no attribute 'CUBIC'
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/tornado/web.py", line 1786, in _execute
    result = await result
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 52, in get
    await self.current(camera_id)
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 110, in current
    picture = mediafiles.get_current_picture(
  File "/usr/local/lib/python3.9/dist-packages/motioneye/mediafiles.py", line 1030, in get_current_picture
    image.thumbnail((width, height), Image.CUBIC)
AttributeError: module 'PIL.Image' has no attribute 'CUBIC'
   ERROR: 500 GET /picture/4/current/?_=1688373828803&width=0.81&_username=admin&_signature=1ed2a6517425a57ab38227d9d2d5791db81597e3 (172.17.0.1) 0.70ms
   ERROR: module 'PIL.Image' has no attribute 'CUBIC'
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/tornado/web.py", line 1786, in _execute
    result = await result
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 52, in get
    await self.current(camera_id)
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 110, in current
    picture = mediafiles.get_current_picture(
  File "/usr/local/lib/python3.9/dist-packages/motioneye/mediafiles.py", line 1030, in get_current_picture
    image.thumbnail((width, height), Image.CUBIC)
AttributeError: module 'PIL.Image' has no attribute 'CUBIC'
   ERROR: 500 GET /picture/8/current/?_=1688373828803&width=0.81&_username=admin&_signature=29e7ecd326d15f1b61bce8a6a8ac2b04470d7ee4 (172.17.0.1) 0.53ms
   ERROR: module 'PIL.Image' has no attribute 'CUBIC'
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/dist-packages/tornado/web.py", line 1786, in _execute
    result = await result
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 52, in get
    await self.current(camera_id)
  File "/usr/local/lib/python3.9/dist-packages/motioneye/handlers/picture.py", line 110, in current
    picture = mediafiles.get_current_picture(
  File "/usr/local/lib/python3.9/dist-packages/motioneye/mediafiles.py", line 1030, in get_current_picture
    image.thumbnail((width, height), Image.CUBIC)
AttributeError: module 'PIL.Image' has no attribute 'CUBIC'
   ERROR: 500 GET /picture/9/current/?_=1688373828803&width=0.81&_username=admin&_signature=c830acb9f9cdb00fd524833a7ad8306ab94f9e5b (172.17.0.1) 0.50ms

@spupuz
Copy link

spupuz commented Jul 3, 2023

what can i do?

@spupuz
Copy link

spupuz commented Jul 3, 2023

reconfigured and reimported the conf files for the cam and it worked.

@jwhite
Copy link

jwhite commented Sep 5, 2023

Is there currently a way to verify that I am indeed running 'dev' and not the old "0.42.1"

I see above that the version string is not being updated.

@zagrim
Copy link
Collaborator

zagrim commented Sep 11, 2023

@jwhite I don't know what you are referring to with "above", but I can confirm that the version show both in UI and in log is 0.43.0 for the current dev version.

@jwhite
Copy link

jwhite commented Sep 12, 2023

@zagrim "above" refers to earlier in this thread. Is it true that 0.43.0 could also signify the last release version that is a non 'dev' version?

@zagrim
Copy link
Collaborator

zagrim commented Sep 13, 2023

I don't think it could, and I still do not see where that would be suggested. However, in the python2 branch, which is the based on the old dev branch before migration to Python3, we still have 0.42.1 in https://github.com/motioneye-project/motioneye/blob/python2/motioneye/__init__.py
Whereas in current dev branch we have 0.43.0 there: https://github.com/motioneye-project/motioneye/blob/dev/motioneye/__init__.py

@brjhaverkamp
Copy link

Hello all,

I have been using motioneye happily on some of my rpi security cameras for a long time now. And I keep an eye on the project to see if a new version is available. But I notice the 0.43 version is now being worked on for about 1.5 years (the age of this thread) What are currently the showstoppers? Are there any? I couldn't find a list of open items.
If not, would it be possible to make 0.43 official ( or after a round of RCs). That would get the ball rolling, possibly attract more attention, support and developers that want to contribute that are now holding off.

@spupuz
Copy link

spupuz commented Dec 5, 2023

Hello all,

I have been using motioneye happily on some of my rpi security cameras for a long time now. And I keep an eye on the project to see if a new version is available. But I notice the 0.43 version is now being worked on for about 1.5 years (the age of this thread) What are currently the showstoppers? Are there any? I couldn't find a list of open items. If not, would it be possible to make 0.43 official ( or after a round of RCs). That would get the ball rolling, possibly attract more attention, support and developers that want to contribute that are now holding off.

Totally agree as a user it's frustrating do not understand which are the changes

@zagrim
Copy link
Collaborator

zagrim commented Dec 6, 2023

@brjhaverkamp @spupuz Regarding the changes: In the sidebar of this issue there is a linked milestone: https://github.com/motioneye-project/motioneye/milestone/1 which contains still plenty of open issues and some features that have been planned to be included in 0.43 release (libcamera support and Motion 4.4 support perhaps being the biggest ones). In the milestone view one can also see the closed issues (ie. completed changes in current dev branch).

@spupuz
Copy link

spupuz commented Dec 6, 2023

@brjhaverkamp @spupuz Regarding the changes: In the sidebar of this issue there is a linked milestone: https://github.com/motioneye-project/motioneye/milestone/1 which contains still plenty of open issues and some features that have been planned to be included in 0.43 release (libcamera support and Motion 4.4 support perhaps being the biggest ones). In the milestone view one can also see the closed issues (ie. completed changes in current dev branch).

yeah ok but why not realease this 0.43.0 an then update the various fixes and implementartion with additional releases?
in that way the version always remain 0.4.3.

@nullr0ute
Copy link

be included in 0.43 release (libcamera support and Motion 4.4 support perhaps being the biggest ones). In the milestone view one can also see the closed issues (ie. completed changes in current dev branch).

I think some of those big features like libcamera are great but could also be a 0.44 release. With the last release not supporting python3 and py2 being EOL 4 years ago personally I think splitting the release into two would be useful for a lot of users. Perfect is the enemy of good :)

@zagrim
Copy link
Collaborator

zagrim commented Dec 7, 2023

Yeah, considering the time it has taken for 0.43 to get to its current state (with also Motion 4.4 support supposedly working but not likely very thoroughly tested), it might be a good idea to split out some of the stuff from that milestone (especially libcamera support seems to be a tough one). I wonder what @MichaIng thinks of that.

Nonetheless, recently there hasn't been much any progress here due to lack of time/energy from anyone (myself included) to move this forward, and getting 0.43 released would require some effort just for the releasing related things.

@brjhaverkamp
Copy link

Hi Zagrim,

Thanks for your thoughts. I see you are also contemplating to release. From my side I would motion (pun intended) to indeed move what is not ready yet as much as possible (libcamera, Motion 4.4) to milestone 0.44 and release 0.43 as soon as possible.
The move to python3 is a big step that will be appreciated by a lot of users.

Indeed there hasn't been a lot of progress in the past months. A release might even draw some new people to add to the effort.
That would be good for the project in general. Atm, I feel a lot of users are currently holding back and sticking to the last stable release ( myself included)

@Michalng, I am curious to hear your thoughts.

Bert

@MichaIng
Copy link
Member Author

Sorry for being mostly inactive lately. I am busy with my main project, the limited spare time I can afford.

motion 4.4 up to 4.6 should work well. All options motionEye sets are adjusted for these versions, and there were no changes after v4.4. But when custom options were added, those need to be updated manually to match the new motion version. Here is a nice overview: https://motion-project.github.io/motion_config.html#Configuration_OptionsAlpha

motionEye 0.43 is not perfect at all, but already much better than the last 0.42 release IMO. Alone Python 3 support, where Python 2 is often not available anymore on modern distros, is a huge benefit, and many bug fixes have been made as well, at least much more than new bugs introduces 😉. Hence, to being things forward, I agree that we should push at least a beta/pre-release to PyPI, to make installing it easier and get more attention. Then at best focusing on fixing the the most urgent bugs, ignoring new features/enhancements for now (unless someone has time and passion to work on a specific topic, of course), and get a "stable" (formally) release out soon as well.

I prepared a PyPI release workflow a while ago: #2675
The only question I was waiting for feedback about:

  • The a1 version suffix automatically leads to a release marked as "pre-release" on PyPI, and requires the pip --pre flag to be picked (unless the version is defined explicitly). That is perfect as a first step.
  • Pre-releases however logically is handled as lower version than the final release without the suffix. So if one has "0.43.0" installed currently, "0.43.0a1" would not be pulled with pip install -U --pre motioneye.
  • Shall we hence bump the version string to "0.43.1a1" or "0.43.1b1" to make the upgrade easier for those which installed "0.43.0" from the dev branch currently?

In the same turn, I would create a "beta" branch from the "dev" branch and a pre-release tag here on GitHub. The release workflow could then be triggered not only manually, but also automatically once a GitHub release tag is created.

@brjhaverkamp
Copy link

Hi MichaIng

I saw you were very active this year on github in other projects. Too much to do.. Hop you have some time to spare for this. i can't oversee the intricacies of the alpha and beta naming unfortunately. From who do you expect/hope for feedback?

But if this is a more commonly used scheme, I would propose to just move forward with it. #2675 Is exactly a year old today, if I read correctly. So you can argue that if someone objected or had comments, there was ample time.

I am not very versed in github nor programming, but I am a tinkerer and hobbyist using motioneye(os). Is there is anything I can do to help?
Regards,

Bert

@zagrim
Copy link
Collaborator

zagrim commented Dec 23, 2023

I don't have any specific experience on which to rely on for this versioning discussion, but I think it's perfectly fine to skip over 0.43.0 in official releases if that makes it easier to move to a useful versioning strategy with alphas and betas etc and still supporting easy upgrades from all the people who've already installed 0.43.0 from the dev branch.

Unless anyone can state any reason why NOT to do so, IMO releasing 0.43.1a1 (or...b1) would be ok as the next step. And as for branching, whatever makes it easy to do pre-releases when needed 😃

@MichaIng
Copy link
Member Author

Okay, and you are right of course, that going forward at some point is better than waiting forever for feedback 😄. Tonight I'll do an 0.43.1a1 to test.pypi.org as a final test for the upload/release workflow, then dump to 0.43.1b1, update the README (install via pip install --pre motioneye), branch off a beta branch and do an upload/release to pypi.org.

@MichaIng
Copy link
Member Author

MichaIng commented Dec 24, 2023

Pre-release done on GitHub: https://github.com/motioneye-project/motioneye/releases/tag/0.43.1b1
Upload to PyPI done: https://pypi.org/project/motioneye/0.43.1b1/

@MichaIng
Copy link
Member Author

MichaIng commented Dec 24, 2023

Can someone help me with the Docker workflow: https://github.com/motioneye-project/motioneye/blob/dev/.github/workflows/docker.yml

  • Warning: 0.43.1b1 is not a valid semver. More info: https://semver.org/: https://github.com/motioneye-project/motioneye/actions/runs/7315066439
    We would need to add a hyphen before potential (a|b|rc)[0-9] suffixes. But where does the metadata-action actually get the version string from? Just from the Git branch or tag? Not sure how to solve this when we need a hyphen for Docker but must not have a hyphen for PyPI. But I guess there is a way to override the string. Or we just do not generate versioned Docker containers for pre-releases, but keep only updating the edge release and in case add a beta one?
  • We currently hardcode type=edge,branch=dev, which we would want to adjust based on the branch/tag it was pushed to? Or are these type= lines pattern which, if they match the branch or tag of the GitHub event, create a respective Docker release tag? Like:
    • type=edge,branch=dev: If GitHub branch matches dev, apply Docker tag edge.
    • type=semver,pattern={{version}}: Else, create semantic version tag from the Git tag.
    • type=semver,pattern={{major}}.{{minor}}: And one from only major.minor.
    • And we hence could add type=beta,branch=beta or something like that?

Merry Christmas to everyone 🙂. I will be back in a few days.

@nullr0ute
Copy link

* `Warning: 0.43.1b1 is not a valid semver. More info: https://semver.org/`: 

Maybe just do it as 0.43.1 and if there's bugs to be fixed just do .2 and .3 etc

@MichaIng
Copy link
Member Author

Done already 😉. For now we'll just raise b1 => b2 etc until we think it is fine to do a stable release. Then as you said, increasing the patch version.

@MichaIng MichaIng changed the title motionEye v0.43.0 release motionEye v0.43.1 release Dec 31, 2023
@spupuz
Copy link

spupuz commented Mar 9, 2024

I still continue to get the same version each time it update

@MichaIng
Copy link
Member Author

MichaIng commented Mar 9, 2024

The latest pre-release is 0.43.1b1: https://pypi.org/project/motioneye/#history
Assure to use the --pre flag with pip to pull pre-releases.

@spupuz
Copy link

spupuz commented Mar 9, 2024

I'm using docker

@MichaIng
Copy link
Member Author

MichaIng commented Mar 9, 2024

This image? https://github.com/motioneye-project/motioneye/pkgs/container/motioneye
Indeed this is edge version, rebuilt on every merged commit to dev branch, while we raise the version string only when merging to beta or main (release) branch.

@spupuz
Copy link

spupuz commented Mar 9, 2024

Ok I'm on edge so this is why I see the same version.

@MichaIng
Copy link
Member Author

@ccrisan I wanted to push the new beta to PyPI, but it throws this error:

ERROR    HTTPError: 400 Bad Request from https://upload.pypi.org/legacy/        
         User 'ccrisan' does not have a verified primary email address. Please  
         add a verified primary email before attempting to upload to PyPI. See  
         https://pypi.org/help/#verified-email for more information.  

Could you check your PyPI account? 🙂

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