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

Remove manylinux container from CI #670

Merged
merged 3 commits into from
Jun 24, 2021
Merged

Remove manylinux container from CI #670

merged 3 commits into from
Jun 24, 2021

Conversation

Tombana
Copy link
Collaborator

@Tombana Tombana commented Jun 24, 2021

What do these changes do?

In our private fork we had problems where the manylinux container is too outdated for our unittests. Combined with the fact that it doesn't actually test anything (the tested code does not really overlap with the converter, which is the only thing that we use the manylinux container for), it makes more sense to remove the container from CI.

How Has This Been Tested?

CI

@Tombana Tombana requested a review from a team June 24, 2021 17:05
- name: Run TFLite Ops and Core CC Unit Tests
run: ./bazelisk test larq_compute_engine/tests:cc_tests --copt=-O2 --distinct_host_configuration=false --test_output=all
- name: Install pip dependencies
run: pip install numpy --no-cache-dir
Copy link
Member

Choose a reason for hiding this comment

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

I think we also used to also need six for TF to build properly. Not sure if this is still the case.

Suggested change
run: pip install numpy --no-cache-dir
run: pip install numpy six --no-cache-dir

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the tip, lets see what CI does.

Copy link
Member

@lgeiger lgeiger left a comment

Choose a reason for hiding this comment

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

This also decreased the build time from ~18min to around ~13min 🚀

@lgeiger lgeiger merged commit ef32e8f into main Jun 24, 2021
@lgeiger lgeiger deleted the manylinux_ci branch June 24, 2021 17:49
@lgeiger lgeiger added the internal-improvement Internal Improvements and Maintenance label Jul 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal-improvement Internal Improvements and Maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants