-
Notifications
You must be signed in to change notification settings - Fork 15
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
Tools transition prepare release #91
Changes from 111 commits
082974d
e332ab5
831ca88
27f2c88
5fcd72b
2eae7db
ffeeaa4
6c7332f
02f4452
271e0d4
6f2f692
0d5079a
09fcf54
eddb078
e8318ad
a98f448
28ffcd1
9189081
b34d6e5
cb83901
a325539
476ab31
72605a5
f353723
7e86b5d
db00b6a
03f4e8f
69f06f0
9cf2e12
6fa30fe
eaa0484
094699d
25d7ebb
b35c6ef
b4078c6
3c95b3d
365dede
ec33dba
a737746
0d7ba17
fecb105
eb6ea07
394fae4
3e039cf
d9e267c
4fa9f9d
ad2bffc
fb4199e
3855dd2
0b47a81
ef50082
46c07f2
4aa84b4
9bff43c
b015bf6
872ff5c
22fc213
8a706d0
39606ba
e55710c
8698ee1
1bb9bea
b4d1761
b68a5bc
c13b72c
a2e8aef
85dc231
acf19f7
3aa3f98
a956c2a
1330096
0b9b606
ffb1142
c40558f
217ccf2
8461511
1273fe2
89c0e63
b89e7eb
14159a4
551fade
b869041
48ff45c
d75a8cf
e62e3bc
a85e2f2
34b2514
7e72c1f
6860248
901ea18
fa1005a
9193cf6
cd390b6
addc33e
7307718
634c931
5e1e907
9c1c198
ae37809
080f34a
616f253
48768d9
ee70e54
3ba99ef
bfffcb8
3aac329
81a4413
db1b32e
5fbb9bc
1d4ec86
2abe570
f91c2dc
25ff7c1
64aec37
c53aa9c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,9 @@ name: Tool Tests | |
|
||
env: | ||
GH_TOKEN: ${{ secrets.GH_TOKEN }} | ||
MICROSOFT_EMAIL: [email protected] | ||
USER_NAME: Gurkan Indibay | ||
MAIN_BRANCH: all-citus | ||
|
||
on: | ||
push: | ||
|
@@ -25,10 +28,14 @@ jobs: | |
steps: | ||
- name: Checkout repository | ||
uses: actions/checkout@v2 | ||
- name: Define git credentials | ||
run: git config --global user.email "${MICROSOFT_EMAIL}"&& git config --global user.name "${USER_NAME}" | ||
- name: Install package dependencies | ||
run: sudo apt install libcurl4-openssl-dev libssl-dev | ||
- name: Install python requirements | ||
run: python -m pip install -r python/requirements.txt | ||
- name: Execute unit tests | ||
run: python -m pytest -q python/tests/test_update_package_properties.py | ||
run: python -m pip install -r packaging_automation/requirements.txt | ||
- name: Execute unit tests for "Update Package Properties" | ||
run: python -m pytest -q packaging_automation/tests/test_update_package_properties.py | ||
- name: Execute unit tests for "Prepare Release" | ||
run: python -m pytest -q packaging_automation/tests/test_prepare_release.py | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -0,0 +1,79 @@ | ||||
# **Prepare Release Usage** | ||||
|
||||
prepare-release.py script performs the pre-packaging configurations in citus/citus-enterprise projects. | ||||
|
||||
## Installation | ||||
|
||||
Before using script, you need to make sure that Python > 3.8 is installed in your system. | ||||
|
||||
### Clone Tools Repository | ||||
|
||||
git clone https://github.com/citusdata/tools.git | ||||
|
||||
Enter 'tools' directory | ||||
|
||||
``` console | ||||
cd tools | ||||
``` | ||||
|
||||
### Install Required Python Libraries | ||||
|
||||
Verify pip installation | ||||
|
||||
``` console | ||||
python -m pip --version | ||||
``` | ||||
Output should be like following | ||||
|
||||
``` console | ||||
pip 21.1.2 from /home/vagrant/.local/lib/python3.8/site-packages/pip (python 3.8) | ||||
``` | ||||
|
||||
If you get error, you should first install pip | ||||
``` console | ||||
sudo apt install python3-pip | ||||
``` | ||||
Install the required libraries to execute the script | ||||
``` console | ||||
python -m pip install -r packaging_automation/requirements.txt | ||||
``` | ||||
If all the steps above completed successfully , you are ready for script execution | ||||
|
||||
## Script Usage | ||||
|
||||
Script can be used for either major release (i.e. third digit of release is '0' e.g. 10.1.0) or | ||||
patch release (i.e. third digit of release is other than '0' e.g. 10.0.4). | ||||
|
||||
### Available flags | ||||
|
||||
**--gh_token:** Personal access token that is authorized to commit citus/citus-enterprise projects. (Required) | ||||
|
||||
**--prj_name:** Project to be released. Allowed values 'citus' and 'citus-enterprise (Required) | ||||
|
||||
**--prj_ver:** Upcoming version to be used for release. should include three level of digits separated by dots, e.g: 10.0.1 | ||||
(Required) | ||||
|
||||
**--main-branch:** Branch to be used as base to be used for configuration changes. There is no need for base scenario. | ||||
onurctirtir marked this conversation as resolved.
Show resolved
Hide resolved
|
||||
This flag can be used for testing purposes. If not used, default branch value is used; i.e. for 'citus' 'master, for 'citus-enterprise' 'enterprise-master' | ||||
|
||||
**--is_test:** If used, branches would not be pushed remote repository and created release branches would be prefixed with 'test'. Default value is False | ||||
|
||||
**--cherry_pick_enabled:** Available only for patch release. If used, --earliest_pr_date flag also should be used.Gets all PR's with 'backport' label created after earliest_pr_date | ||||
|
||||
**--earliest_pr_date:** Used with --cherry-pick-enabled flag. Date format is 'Y.m.d' e.g 2012.01.21. PR's merged after this date would be listed and cherry-picked. | ||||
|
||||
**--schema-version:** Available only for patch release. If used, schema version in citus.control file would be updated. | ||||
|
||||
###Example Usage | ||||
|
||||
####Major | ||||
``` console | ||||
python -m packaging_automation.prepare_release --gh_token <your-personal-token> --prj_name citus --prj_ver 10.1.0 | ||||
``` | ||||
#### Patch | ||||
``` console | ||||
python -m packaging_automation.prepare_release --gh_token <your-personal-token> --prj_name citus-enterprise --prj_ver 10.0.4 --schema_version 10.0-5 | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's also add here There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is no need to add main-branch because main branch is set by project_name if not given as parameter tools/packaging_automation/prepare_release.py Line 563 in 25ff7c1
|
||||
``` | ||||
|
||||
|
||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sharing some additional comments from my manual testing:
Minor release flow seems pretty fine.
Major release flow:
Release branch creation works fine.
But the changes made for master branch doesn't seem to be correct. Please check with the commit that I shared before
Getting some errors after running the script for the second time:
For the missing/incorrect parameters, I would expect throwing errors beforehand, but for the most, we throw the error in very deeper levels (like when using github api or when splitting a string etc.).
Let's make some correctness checks for them at the very beginning of our main entrypoint.
I think we can have a function to do those checks such as
check_args
or something.In
initialize_env
function, we should decide which repo to clone according toprj_name
arg.It can be either "citus" or "citus-enterprise". So I think we can use
add_arg(..choices=..)
for that.For
--is_test
&cherry_pick_enabled
params, I think we can useadd_arg(..action='store_true'..
, instead of requiring to pass "True" string.For
schema_version
, I think we should error if it's passed for major release.But we should be okay if it is passed or not passed for patch release.
I think we should set
add_arg(..required=true
..), forprj_ver
,main_branch
&gh_token
.We should error if either of
cherry_pick_enabled
orearliest_pr_date
is set for major release.But we should be okay if it is passed or not passed for patch release.
We should error if
cherry_pick_enabled
is set to true butearliest_pr_date
is not given.Can we simplify
earliest_pr_date
format, like just'%Y.%m.%d
? I don't think we need to know hour, minute etc. ever.Seems that we currently don't use
exec_path
arg, can we remove that ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I addressed all of them except the action="store_true" parameters because when this parameter is used, it can not be set by user. It sets a constant variable not suitable for user-defined parameters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't actually think it cannot be set by user.
Assuming
is_test
arg, usingstore_true
means:If user doesn't add
--is_test
, thenis_test
would automatically evaluate tofalse
, so we will disable test mode.If user puts
--is_test
into the args (without any additional strings), thenargparese
should interpretis_test
astrue
.https://stackoverflow.com/a/8203679/13370386
Is there any reason for not using that ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hadn't totally grasped the parameter. As you referenced, I understood and changed that way thanks 👍