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

Revamp appu/migrate appu #100

Open
wants to merge 3 commits into
base: revamp
Choose a base branch
from
Open

Revamp appu/migrate appu #100

wants to merge 3 commits into from

Conversation

ifosch
Copy link
Member

@ifosch ifosch commented Mar 16, 2025

This PR introduces an appu subcommand to APPU 2, with the same features as old appu program/container. This also introduces a first version for a container image for APPU 2.

This proposal takes one first step to replace old appu tooling with a new one. In theory, this could replace the Edit and upload the audio file step in the publishing pipeline.

Check out the commit messages for more information regarding:

  • New subcommand features and tests added
  • Changes in dependencies, caused by upgrading to Python 3.13
  • Changes in selection of CI pipelines steps
  • Trick added to prevent weird output, caused by pydub's not supporting Python 3.12+
  • Tests and container usage.

Further work in this direction, pending discussions, could be:

  • Replace epidator
  • Replace feedupdater
  • Replace uploader
  • Include publishing Youtube to the tool
  • Add a validation step
  • Add more possible editions to master recording

@ifosch ifosch marked this pull request as ready for review March 16, 2025 06:49
@ifosch ifosch force-pushed the revamp-appu/migrate-appu branch from 7588e7b to 6f23ca4 Compare March 16, 2025 06:50
@ifosch ifosch marked this pull request as draft March 16, 2025 06:51
@ifosch ifosch force-pushed the revamp-appu/migrate-appu branch 5 times, most recently from d0c4a92 to 824f08f Compare March 16, 2025 07:18
Ignasi Fosch added 3 commits March 16, 2025 10:06
This command provides the features we have in the old
appu's Python code:

* Loads configuration for the episode
* downloads the master audio specified in the episode configuration
* normalizes the audio in the recording
* downloads the intro file and the cover image
* splits the intro file
* concatenates the intro, the recording, and the outro all together
* exports the track, with the corresponding ID3 tags, including the
cover image
* uploads the track to the destination for the edited recording

This change also includes using `audioop-lts`, which is a `pydub`'s
dependency in Python 3.13.
This adds `pytest` testing for the new `appu` subcommand, and the code
provided. Those tests are also included in the `test` pipeline to test
the subcommand within the CI pipeline. This uses a container to run
the tests, so it also validates building the APPU 2's container.

All the other steps in the CI pipeline, are selected to skip when
requesting to pull from a branch with `revamp` in the name. Ideally,
this should prevent issues between revamping changes and other
possible changes coming in.

The `PYTHONWARNINGS` environment variable in the Dockerfile is
provided to prevent weird output caused by [an issue with `pydub` and
regular expression support for Python
3.12](jiaaro/pydub#801).
@ifosch ifosch force-pushed the revamp-appu/migrate-appu branch from 824f08f to 9ef889c Compare March 16, 2025 09:22
@ifosch ifosch marked this pull request as ready for review March 16, 2025 09:23
@ifosch ifosch changed the title [DO NOT MERGE] Revamp appu/migrate appu Revamp appu/migrate appu Mar 16, 2025
@ifosch
Copy link
Member Author

ifosch commented Mar 16, 2025

@davidpoblador This PR, based on revamp branch, is, IMO, ready for merging. Happy to hear your feedback on this, or discussing any detail.

@davidpoblador
Copy link

This looks, good! I will review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants