[3.0] [flink-cdc-dist] Collect all distro resources into a TAR#2636
Merged
PatrickRen merged 7 commits intoapache:masterfrom Dec 1, 2023
Merged
[3.0] [flink-cdc-dist] Collect all distro resources into a TAR#2636PatrickRen merged 7 commits intoapache:masterfrom
PatrickRen merged 7 commits intoapache:masterfrom
Conversation
GOODBOY008
force-pushed
the
master-3.0
branch
from
88cbd6c to
6d2bc12
Compare
GOODBOY008
changed the title
GOODBOY008
force-pushed
the
master-3.0
branch
from
32ce5a6 to
59ff733
Compare
GOODBOY008
changed the title
GOODBOY008
force-pushed
the
master-3.0
branch
from
59ff733 to
78bcda5
Compare
GOODBOY008
force-pushed
the
master-3.0
branch
from
78bcda5 to
9fdb79d
Compare
PatrickRen
reviewed
Nov 29, 2023
Contributor
PatrickRen
left a comment
PatrickRen
left a comment
There was a problem hiding this comment.
@GOODBOY008 Thanks for the PR! I left some comments.
| <dependencySets> | ||
| <dependencySet> | ||
| <unpack>false</unpack> | ||
| <scope>runtime</scope> |
Contributor
There was a problem hiding this comment.
I found some suspicious JARs in the final tarball:
- Awaitility: This should be a test dependency, and should not be a part of the distribution. We need to change its scope to
testin pom. - Hamcrest: This is defined as test scope in out project, but brought into the project as
compileby Awaitility. I guess it will disappear once we change Awaitility to test scope. - flink-cdc-dist: This is a JAR without any class file inside. We can just skip it when assembling.
- commons-cli: What about packaging it directly into
flink-cdc-cli?
Things missing:
- Log4j JAR and config file: Users cannot have logging locally without them
Contributor
There was a problem hiding this comment.
About log4j I made a second thought, that maybe it's better to use log4j JARs under FLINK_HOME directly. We just need to provide a log4j configuration file for printing Flink CDC CLI and composer logs to a local file. Sorry for that!
GOODBOY008
force-pushed
the
master-3.0
branch
from
9fdb79d to
e9534dd
Compare
- flink-cdc-dist should depends on all Flink CDC modules to enforce the building order - Package a uber JAR with all Flink CDC classes and dependencies - Remove the profile to make packaging flink-cdc-dist as the default behavior
- Keep a directory under target for easier validation - Add logic for copying flink-cdc-dist uber JAR to lib
Contributor
|
I pushed 4 commits to the PR just now:
@GOODBOY008 Could you take a look at the PR? I checked the packaged tarball and it should work as expected now. |
Member
Author
|
@PatrickRen I tested on my local, works as we want. Thank you for complete my rest work and address issue. 👍 |
leonardBang
approved these changes
Dec 1, 2023
Contributor
leonardBang
left a comment
leonardBang
left a comment
There was a problem hiding this comment.
Thanks @GOODBOY008 and @PatrickRen for the great work, I went through the files, LGTM
2 tasks
e-mhui
pushed a commit
to e-mhui/flink-cdc-connectors
that referenced
this pull request
Dec 2, 2023
…e#2636) This closes apache#2621 apache#2622 Co-authored-by: Qingsheng Ren <renqschn@gmail.com>
ChaomingZhangCN
pushed a commit
to ChaomingZhangCN/flink-cdc
that referenced
this pull request
Jan 13, 2025
…e#2636) This closes apache#2621 apache#2622 Co-authored-by: Qingsheng Ren <renqschn@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Close #2621 #2622