Skip to content

Conversation

@zentol
Copy link
Contributor

@zentol zentol commented Oct 5, 2022

Fix for flink-fs-hadoop-shaded, flink-s3-fs-base and flink-table-planner-loader-bundle not being covered by license checks as they got auto-ignored due to not being deployed.
The checker now only ignores non-deployed modules if they are not bundled by another deployed module.

Note that this approach will not work if a non-deployed module is bundled by another non-deployed module which is then bundled by a deployed module.
This is currently not a problem though.

I noticed this problem when I saw this in the CI output of a build, despite the NOTICE being correct:

	flink-table-planner-loader-bundle:
		 These issues are mistakes that aren't legally problematic. They SHOULD be fixed at some point, but we don't have to: 
			Dependency org.scala-lang:scala-compiler:2.12.7 is not bundled, but listed.
			Dependency org.scala-lang:scala-library:2.12.7 is not bundled, but listed.
			Dependency org.scala-lang:scala-reflect:2.12.7 is not bundled, but listed.

What happened is that the NOTICE was still found, but we already threw out the information about bundles dependencies for that module.

We really need some tests for this...

@zentol zentol requested a review from alpinegizmo October 5, 2022 12:16
@flinkbot
Copy link
Collaborator

flinkbot commented Oct 5, 2022

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

@zentol
Copy link
Contributor Author

zentol commented Oct 5, 2022

Let's maybe first get #20970 in before making further changes.

Copy link
Contributor

@alpinegizmo alpinegizmo left a comment

Choose a reason for hiding this comment

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

Looks good. Just a trivial typo.

The number of modules with bundled dependencies can exceed the number of deployed modules.
Retain information for bundled Flink dependencies and filter them later on where appropriate.
@zentol zentol merged commit 01d7b37 into apache:master Oct 12, 2022
@zentol zentol deleted the 29508 branch October 12, 2022 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants