Skip to content

Fix ci build #1390

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

Merged
merged 50 commits into from
May 13, 2025
Merged

Fix ci build #1390

merged 50 commits into from
May 13, 2025

Conversation

JoshuaSBrown
Copy link
Collaborator

@JoshuaSBrown JoshuaSBrown commented Apr 23, 2025

PR Description

Tasks

  • - A description of the PR has been provided, and a diagram included if it is a new feature.
  • - Formatter has been run
  • - CHANGELOG comment has been added
  • - Labels have been assigned to the pr
  • - A reviwer has been added
  • - A user has been assigned to work on the pr
  • - If new feature a unit test has been added

Summary by Sourcery

Update CI and dependency configurations to improve build compatibility and dependency management

Build:

  • Update protobuf dependency to a newer version
  • Replace python3-pkg-resources with python3-setuptools in dependency installation script

CI:

  • Update GitHub Actions workflow to use Ubuntu 24.04 as the runner environment

Copy link

sourcery-ai bot commented Apr 23, 2025

Reviewer's Guide by Sourcery

This PR updates the CI build environment to Ubuntu 24.04 and upgrades the protobuf package version. Additionally, it replaces python3-pkg-resources with python3-setuptools in the dependency installation script.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Updated the CI build environment to Ubuntu 24.04.
  • Changed the runs-on value in the unit-tests.yml workflow from ubuntu-20.04 to ubuntu-24.04.
.github/workflows/unit-tests.yml
Upgraded the protobuf package version.
  • Updated the protobuf package version in requirements.txt from 5.27.1 to 6.30.2.
python/datafed_pkg/requirements.txt
Replaced python3-pkg-resources with python3-setuptools in the dependency installation script.
  • Modified the packages array in install_client_dependencies.sh to include python3-setuptools instead of python3-pkg-resources.
scripts/install_client_dependencies.sh

Possibly linked issues

  • #0: The PR fixes the failing CI build by updating protobuf version, changing OS, and updating requirements.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!
  • Generate a plan of action for an issue: Comment @sourcery-ai plan on
    an issue to generate a plan of action for it.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @JoshuaSBrown - I've reviewed your changes - here's some feedback:

Overall Comments:

  • It would be good to understand why these dependencies are being updated.
Here's what I looked at during the review
  • 🟢 General issues: all looks good
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@JoshuaSBrown JoshuaSBrown linked an issue Apr 23, 2025 that may be closed by this pull request
3 tasks
@JoshuaSBrown JoshuaSBrown added Component: Build Related to the build system Component: CI labels May 7, 2025
@@ -66,7 +66,7 @@ COPY ./scripts/install_authz_dependencies.sh ${BUILD_DIR}/scripts/
COPY ./scripts/copy_dependency.sh ${BUILD_DIR}/scripts/

RUN ${BUILD_DIR}/scripts/generate_datafed.sh
RUN DEBIAN_FRONTEND=noninteractive TZ=Etc/UTC ${BUILD_DIR}/scripts/install_authz_dependencies.sh unify
RUN DEBIAN_FRONTEND=noninteractive TZ=Etc/UTC ${BUILD_DIR}/scripts/install_authz_dependencies.sh
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nedvedba why this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks like we weren't installing any of the dependencies with the 'unify' option.

Comment on lines +67 to +82
install_python() {
if [ ! -e "${DATAFED_DEPENDENCIES_INSTALL_PATH}/.python_installed-${DATAFED_PYTHON_VERSION}" ]; then
# Check if the deadsnakes repository has already been added to avoid issues with gpg
if ! grep -qr '^deb .\+deadsnakes' /etc/apt/sources.list.d/; then
"$SUDO_CMD" apt update
"$SUDO_CMD" apt install -y software-properties-common
"$SUDO_CMD" add-apt-repository -y ppa:deadsnakes/ppa
"$SUDO_CMD" apt update
fi

"$SUDO_CMD" apt install -y "python${DATAFED_PYTHON_VERSION}" "python${DATAFED_PYTHON_VERSION}-dev" "python${DATAFED_PYTHON_VERSION}-venv" "python${DATAFED_PYTHON_VERSION}-distutils"

touch "${DATAFED_DEPENDENCIES_INSTALL_PATH}/.python_installed-${DATAFED_PYTHON_VERSION}"
fi
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We probably need a way to install all dependencies that is not distro specific, I really don't like using apt for this it makes things brittle.

@JoshuaSBrown JoshuaSBrown merged commit 06d4aff into devel May 13, 2025
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Build Related to the build system Component: CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] Fix Failing Build Since CI updates
2 participants