The Wayback Machine - https://web.archive.org/web/20220902123600/https://github.com/apache/superset/pull/10848
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

Jump to bottom

Update FOSSA configuration for new requirements layout #10848

Merged
merged 7 commits into from Sep 16, 2020
Merged

Update FOSSA configuration for new requirements layout #10848

merged 7 commits into from Sep 16, 2020

Conversation

robdiciuccio
Copy link
Member

@robdiciuccio robdiciuccio commented Sep 11, 2020

SUMMARY

FOSSA is not accurately checking or failing on license issues with modules in the base requirements. This PR updates the FOSSA config to test base and docker requirements for license issues, and fails the build if any are found.

I've also made the FOSSA report public, so anyone should be able to view.

NOTE: there are currently dependencies in base.txt that do not meet Apache licensing requirements (e.g. mysqlclient and psycopg2-binary), so the check is expected to fail.

TEST PLAN

  • FOSSA should run in CI and should fail on incompatible licenses

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API
@superset-github-bot superset-github-bot bot added the !deprecated-label:org:preset Deprecated label - Use preset-io instead label Sep 11, 2020
willbarrett
willbarrett approved these changes Sep 12, 2020
View changes
Copy link
Member

@willbarrett willbarrett left a comment

LGTM after rebase
@codecov-commenter
Copy link

codecov-commenter commented Sep 15, 2020 •
edited

Codecov Report

Merging #10848 into master will decrease coverage by 0.22%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10848      +/-   ##
==========================================
- Coverage   61.34%   61.12%   -0.23%     
==========================================
  Files         380      380              
  Lines       24068    24083      +15     
==========================================
- Hits        14765    14720      -45     
- Misses       9303     9363      +60
Flag Coverage Δ
#python 61.12% <ø> (-0.23%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/db_engine_specs/mysql.py 79.16% <0.00%> (-12.50%) ⬇️
superset/db_engine_specs/presto.py 73.54% <0.00%> (-8.75%) ⬇️
superset/examples/world_bank.py 97.10% <0.00%> (-2.90%) ⬇️
superset/examples/birth_names.py 97.36% <0.00%> (-2.64%) ⬇️
superset/models/core.py 87.29% <0.00%> (-1.05%) ⬇️
superset/db_engine_specs/base.py 87.26% <0.00%> (-0.42%) ⬇️
superset/views/core.py 74.22% <0.00%> (-0.25%) ⬇️
superset/connectors/sqla/models.py 89.61% <0.00%> (-0.14%) ⬇️
superset/databases/utils.py 78.94% <0.00%> (+0.56%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4502690...d1a95ca. Read the comment docs.
@robdiciuccio
Copy link
Member Author

robdiciuccio commented Sep 15, 2020

Example public FOSSA report:
https://app.fossa.com/projects/custom+11342%2Fgithub.com%2Fapache%2Fincubator-superset/refs/branch/HEAD/c1b5d64859540a08ba74a41bfc3c9a54d506b2e4
@robdiciuccio
Revert "test FOSSA failure"
7d55750 
This reverts commit a97213d.
@robdiciuccio robdiciuccio mentioned this pull request Sep 15, 2020
Merged
6 tasks
craig-rueda
craig-rueda requested changes Sep 15, 2020
View changes
.fossa.yml Outdated
path: .
options:
requirements: ./requirements/base.txt
- name: docker
Copy link
Member

@craig-rueda craig-rueda Sep 15, 2020

Choose a reason for hiding this comment

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

We don't need this check, as the Docker requirements are only used for the "dev" image (which is used in docker-compose)
Copy link
Member Author

@robdiciuccio robdiciuccio Sep 15, 2020

Choose a reason for hiding this comment

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

So, it looks like the "lean" Docker image is using requirements/local.txt? This is confusing.
Copy link
Member

@craig-rueda craig-rueda Sep 15, 2020

Choose a reason for hiding this comment

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

Yep, lean gets "local" and dev gets "docker". When I did this, "local" already existed, so I reused it. I added "docker.txt" in order to append a few more deps for docker-compose.
Copy link
Member Author

@robdiciuccio robdiciuccio Sep 16, 2020

Choose a reason for hiding this comment

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

Ok, removed it for now. May need some reworking after #10875 settles.
@robdiciuccio
Copy link
Member Author

robdiciuccio commented Sep 16, 2020

We should probably run the license check each time code is merged to master, as well as when the requirements files change.
@rusackas rusackas merged commit d74044c into apache:master Sep 16, 2020
19 checks passed
@rusackas rusackas deleted the rd/fossa branch Sep 16, 2020
auxten pushed a commit to auxten/incubator-superset that referenced this issue Nov 20, 2020
@robdiciuccio @auxten
Update FOSSA configuration for new requirements layout (apache#10848)
d77a36f 
* Update FOSSA configuration

* test FOSSA failure

* Update FOSSA files changed regex

* Revert "test FOSSA failure"

This reverts commit a97213d.

* pre-commit linting

* remove docker.txt check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
!deprecated-label:org:preset Deprecated label - Use preset-io instead size/S
5 participants
@robdiciuccio @codecov-commenter @willbarrett @craig-rueda @rusackas