Dynamic Shim Detection for build Process [databricks]
#11308
Conversation
razajafri
requested review from
jlowe,
revans2,
tgravescs,
GaryShen2008,
NvTimLiu and
gerashegalov
as code owners
razajafri
changed the title
build Process [databricks]
Signed-off-by: Raza Jafri <[email protected]>
There was a problem hiding this comment.
Has this been tested for a typical developer build where we're just building for one shim? The build appeared to hang for quite a while (~25secs) at the create-release-properties stage, probably because it's invoking Maven from scratch for every release profile which is not fast. It then subsequently fails in the shimplify stage for the same reasons already identified in the CI premerge workflows. Even if it had proceeded, this seems like it would have likely made the overall build time worse, not better.
|
|
||
| if "scala2.13" in pomFile: | ||
| no_snapshots = list(filter(lambda x: not x.endswith("cdh"), no_snapshots)) | ||
| db_release = list(filter(lambda x: x.endswith("db"), no_snapshots)) |
There was a problem hiding this comment.
nit: new line. try reformat the code for better readability 👍
build/get_buildvers.py
Outdated
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| print(_get_expression(sys.argv[2], sys.argv[1])) |
There was a problem hiding this comment.
nit: it would be nice to include a clear usage message with a if len(sys.argv) != 3 check, as this will be used directly in some scripts
|
I have added profiles so as not to break the prescribed way to build in our CONTRIBUTING guide. |
|
build |
1 similar comment
|
build |
|
checking, let me verify to make sure it works for pre-merge/nightly CIs |
build/get_buildvers.py
Outdated
|
|
||
| if __name__ == "__main__": | ||
| if len(sys.argv) != 3: | ||
| print("get_buildvers.py needs a pom_file location and an buildvers as arguments") |
There was a problem hiding this comment.
Usually tools have a more formal output
| print("get_buildvers.py needs a pom_file location and an buildvers as arguments") | |
| print("usage: get_buildvers.py <pom_file> <buildvers>") | |
| print(' supported buildvers: databricks, no_snapshots, ...') |
We can also probably get this for free with optparse if needed
dist/pom.xml
Outdated
| <phase>generate-sources</phase> | ||
| <configuration> | ||
| <target xmlns:ac="antlib:net.sf.antcontrib"> | ||
| <exec executable="python" outputproperty="included_buildvers"> |
There was a problem hiding this comment.
is it necessary to rely on external python process? We have previously used jython for portability
Line 368 in d2abcd9
|
Verified the nightly CI for scala 2.12 got PASS There're some trivial CI script issue for scala2.13 build and test, I'll fix them today to make it work with this PR |
|
Verified this PR in CI jobs, CI can PASS along with the CI script update: #11427 |
|
@gerashegalov do you have any more concerns here? |
|
build |
This PR removes most of the profiles from pom.xml that we used to maintain the snapshots and such manually
We now call a Python script that parses the pom.xml and creates the snapshots and non-snapshots releases from the release profiles in pom.xml avoiding maintenance of separate lists of snapshots.buildvers/noSnapshots.buildvers/snapshotsScala213.buildvers/noSnapshotsScala213.buildvers properties in pom.xml
Example: To get noSnapshots run
python build/get_buildvers.py no_snapshots pom.xml. To get the same for Scala2.13 we need to point to the right pom.xml in the scala2.13 folder i.e.scala2.13/pom.xmlTo display debug info while
shimplify.pyis running add-Ddyn.shim.trace=trueto the build command to display the debug info.