Skip to content
This repository has been archived by the owner on Jul 10, 2024. It is now read-only.

Commit

Permalink
SUBMARINE-1008. Submarine python code formatter and linter
Browse files Browse the repository at this point in the history
### What is this PR for?
Currently, submarine python code format doesn't work well.
1. Replace `yapf` with `black`
2. Upgrade `isort`
3. Use `flake8` linter
4. Format the code in dev-support, submarine-sdk and website
5. Only check lint in dev-support, submarine-sdk since python code in website has its' style. (import not at top of file)
6. Update the SDK development guide

### What type of PR is it?
[Improvement]

### Todos

### What is the Jira issue?
https://issues.apache.org/jira/browse/SUBMARINE-1008

### How should this be tested?
I have updated the ci in GitHub Actions.

### Screenshots (if appropriate)
Original `dev-support/style-check/python/lint.sh` result. I have fixed all of the errors in the below images
![螢幕擷取畫面 2021-09-06 094648](https://user-images.githubusercontent.com/38066413/132158081-bdcfe866-08b6-44fa-a5d2-23f9cec3aaa9.png)
![image](https://user-images.githubusercontent.com/38066413/132158044-ae18abed-469f-4cf9-a866-b0144a8f2698.png)

### Questions:
* Do the license files need updating? No
* Are there breaking changes for older versions? Yes
* Does this need new documentation? Yes

Author: KUAN-HSUN-LI <[email protected]>

Signed-off-by: Kevin <[email protected]>

Closes #736 from KUAN-HSUN-LI/SUBMARINE-1008 and squashes the following commits:

157ac0e [KUAN-HSUN-LI] SUBMARINE-1008. Remove useless file and update the 0.6.0 docs
633cdc1 [KUAN-HSUN-LI] add License
643bb94 [KUAN-HSUN-LI] SUBMARINE-1008. Update development guide
0638f67 [KUAN-HSUN-LI] SUBMARINE-1008. Update linter in ci
8892b26 [KUAN-HSUN-LI] SUBMARINE-1008. Update develop guide
4e086e8 [KUAN-HSUN-LI] SUBMARINE-1008. Format with isort and black. Check through flake8
7f679fd [KUAN-HSUN-LI] SUBMARINE-1008. Setup black, isort and flake8
  • Loading branch information
KUAN-HSUN-LI authored and pingsutw committed Sep 6, 2021
1 parent f621a32 commit 6fa91e3
Show file tree
Hide file tree
Showing 121 changed files with 4,640 additions and 4,474 deletions.
19 changes: 19 additions & 0 deletions .flake8
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#

[flake8]
max-line-length = 100
4 changes: 2 additions & 2 deletions .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ jobs:
pip install --no-cache-dir torch==1.5.0
pip install --no-cache-dir ./submarine-sdk/pysubmarine/.
pip install -r ./submarine-sdk/pysubmarine/github-actions/test-requirements.txt
pip install -r ./submarine-sdk/pysubmarine/github-actions/lint-requirements.txt
pip install -r ./dev-support/style-check/python/lint-requirements.txt
- name: Check python sdk code style
run: ./submarine-sdk/pysubmarine/github-actions/lint.sh
run: ./dev-support/style-check/python/lint.sh
- name: Run unit test
run: pytest --cov=submarine -vs -m "not e2e"
integration:
Expand Down
147 changes: 87 additions & 60 deletions dev-support/cicd/merge_submarine_pr.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,12 @@
import re
import subprocess
import sys

import urllib2

try:
import jira.client

JIRA_IMPORTED = True
except ImportError:
JIRA_IMPORTED = False
Expand Down Expand Up @@ -61,27 +63,27 @@
def get_json(url):
try:
return json.load(urllib2.urlopen(url))
except urllib2.HTTPError as e:
print "Unable to fetch URL, exiting: %s" % url
except urllib2.HTTPError:
print("Unable to fetch URL, exiting: %s" % url)
sys.exit(-1)


def fail(msg):
print msg
print(msg)
clean_up()
sys.exit(-1)


def run_cmd(cmd):
print cmd
print(cmd)
if isinstance(cmd, list):
return subprocess.check_output(cmd)
else:
return subprocess.check_output(cmd.split(" "))


def continue_maybe(prompt):
result = raw_input("\n%s (y/n): " % prompt)
result = input("\n%s (y/n): " % prompt)
if result.lower() != "y":
fail("Okay, exiting")

Expand All @@ -90,13 +92,13 @@ def continue_maybe(prompt):


def clean_up():
print "Restoring head pointer to %s" % original_head
print("Restoring head pointer to %s" % original_head)
run_cmd("git checkout %s" % original_head)

branches = run_cmd("git branch").replace(" ", "").split("\n")

for branch in filter(lambda x: x.startswith(BRANCH_PREFIX), branches):
print "Deleting local branch %s" % branch
print("Deleting local branch %s" % branch)
run_cmd("git branch -D %s" % branch)


Expand All @@ -110,31 +112,33 @@ def merge_pr(pr_num, target_ref):

had_conflicts = False
try:
run_cmd(['git', 'merge', pr_branch_name, '--squash'])
run_cmd(["git", "merge", pr_branch_name, "--squash"])
except Exception as e:
msg = "Error merging: %s\nWould you like to manually fix-up this merge?" % e
continue_maybe(msg)
msg = "Okay, please fix any conflicts and 'git add' conflicting files... Finished?"
continue_maybe(msg)
had_conflicts = True

commit_authors = run_cmd(['git', 'log', 'HEAD..%s' % pr_branch_name,
'--pretty=format:%an <%ae>']).split("\n")
commit_date = run_cmd(['git', 'log', '%s' % pr_branch_name, '-1',
'--pretty=format:%ad'])
distinct_authors = sorted(set(commit_authors),
key=lambda x: commit_authors.count(x), reverse=True)
commit_authors = run_cmd(
["git", "log", "HEAD..%s" % pr_branch_name, "--pretty=format:%an <%ae>"]
).split("\n")
commit_date = run_cmd(["git", "log", "%s" % pr_branch_name, "-1", "--pretty=format:%ad"])
distinct_authors = sorted(
set(commit_authors), key=lambda x: commit_authors.count(x), reverse=True
)
primary_author = distinct_authors[0]
commits = run_cmd(['git', 'log', 'HEAD..%s' % pr_branch_name,
'--pretty=format:%h [%an] %s']).split("\n\n")
commits = run_cmd(
["git", "log", "HEAD..%s" % pr_branch_name, "--pretty=format:%h [%an] %s"]
).split("\n\n")

merge_message_flags = []

merge_message_flags += ["-m", title]
if body is not None:
# We remove @ symbols from the body to avoid triggering e-mails
# to people every time someone creates a public fork of Submarine.
if isinstance(body, unicode):
if isinstance(body, re.UNICODE):
merge_message_flags += ["-m", body.encode("utf-8").replace("@", "")]
else:
merge_message_flags += ["-m", body.replace("@", "")]
Expand All @@ -145,27 +149,37 @@ def merge_pr(pr_num, target_ref):

committer_name = run_cmd("git config --get user.name").strip()
committer_email = run_cmd("git config --get user.email").strip()
merge_message_flags += ["-m", "\n" + "Signed-off-by: %s <%s>" % (committer_name, committer_email)]
merge_message_flags += [
"-m",
"\n" + "Signed-off-by: %s <%s>" % (committer_name, committer_email),
]

if had_conflicts:
message = "This patch had conflicts when merged, resolved by\nCommitter: %s <%s>" % (
committer_name, committer_email)
committer_name,
committer_email,
)
merge_message_flags += ["-m", message]

# The string "Closes #%s" string is required for GitHub to correctly close the PR
merge_message_flags += [
"-m",
"Closes #%s from %s and squashes the following commits:" % (pr_num, pr_repo_desc)]
"Closes #%s from %s and squashes the following commits:" % (pr_num, pr_repo_desc),
]
for c in commits:
merge_message_flags += ["-m", c]

run_cmd(['git', 'commit', '--author="%s"' % primary_author, '--date="%s"' % commit_date] + merge_message_flags)
run_cmd(
["git", "commit", '--author="%s"' % primary_author, '--date="%s"' % commit_date]
+ merge_message_flags
)

continue_maybe("Merge complete (local ref %s). Push to %s?" % (
target_branch_name, PUSH_REMOTE_NAME))
continue_maybe(
"Merge complete (local ref %s). Push to %s?" % (target_branch_name, PUSH_REMOTE_NAME)
)

try:
run_cmd('git push %s %s:%s' % (PUSH_REMOTE_NAME, target_branch_name, target_ref))
run_cmd("git push %s %s:%s" % (PUSH_REMOTE_NAME, target_branch_name, target_ref))
except Exception as e:
clean_up()
fail("Exception while pushing: %s" % e)
Expand All @@ -178,7 +192,7 @@ def merge_pr(pr_num, target_ref):


def cherry_pick(pr_num, merge_hash, default_branch):
pick_ref = raw_input("Enter a branch name [%s]: " % default_branch)
pick_ref = input("Enter a branch name [%s]: " % default_branch)
if pick_ref == "":
pick_ref = default_branch

Expand All @@ -195,11 +209,12 @@ def cherry_pick(pr_num, merge_hash, default_branch):
msg = "Okay, please fix any conflicts and finish the cherry-pick. Finished?"
continue_maybe(msg)

continue_maybe("Pick complete (local ref %s). Push to %s?" % (
pick_branch_name, PUSH_REMOTE_NAME))
continue_maybe(
"Pick complete (local ref %s). Push to %s?" % (pick_branch_name, PUSH_REMOTE_NAME)
)

try:
run_cmd('git push %s %s:%s' % (PUSH_REMOTE_NAME, pick_branch_name, pick_ref))
run_cmd("git push %s %s:%s" % (PUSH_REMOTE_NAME, pick_branch_name, pick_ref))
except Exception as e:
clean_up()
fail("Exception while pushing: %s" % e)
Expand All @@ -222,10 +237,11 @@ def fix_version_from_branch(branch, versions):


def resolve_jira_issue(merge_branches, comment, default_jira_id=""):
asf_jira = jira.client.JIRA({'server': JIRA_API_BASE},
basic_auth=(JIRA_USERNAME, JIRA_PASSWORD))
asf_jira = jira.client.JIRA(
{"server": JIRA_API_BASE}, basic_auth=(JIRA_USERNAME, JIRA_PASSWORD)
)

jira_id = raw_input("Enter a JIRA id [%s]: " % default_jira_id)
jira_id = input("Enter a JIRA id [%s]: " % default_jira_id)
if jira_id == "":
jira_id = default_jira_id

Expand All @@ -244,15 +260,23 @@ def resolve_jira_issue(merge_branches, comment, default_jira_id=""):

if cur_status == "Resolved" or cur_status == "Closed":
fail("JIRA issue %s already has status '%s'" % (jira_id, cur_status))
print ("=== JIRA %s ===" % jira_id)
print ("summary\t\t%s\nassignee\t%s\nstatus\t\t%s\nurl\t\t%s/%s\n" % (
cur_summary, cur_assignee, cur_status, JIRA_BASE, jira_id))
print("=== JIRA %s ===" % jira_id)
print(
"summary\t\t%s\nassignee\t%s\nstatus\t\t%s\nurl\t\t%s/%s\n"
% (
cur_summary,
cur_assignee,
cur_status,
JIRA_BASE,
jira_id,
)
)

versions = asf_jira.project_versions("SUBMARINE")
versions = sorted(versions, key=lambda x: x.name, reverse=True)
versions = filter(lambda x: x.raw['released'] is False, versions)
versions = filter(lambda x: x.raw["released"] is False, versions)
# Consider only x.y.z versions
versions = filter(lambda x: re.match('\d+\.\d+\.\d+', x.name), versions)
versions = filter(lambda x: re.match(r"\d+\.\d+\.\d+", x.name), versions)

default_fix_versions = map(lambda x: fix_version_from_branch(x, versions).name, merge_branches)
for v in default_fix_versions:
Expand All @@ -267,7 +291,7 @@ def resolve_jira_issue(merge_branches, comment, default_jira_id=""):
default_fix_versions = filter(lambda x: x != v, default_fix_versions)
default_fix_versions = ",".join(default_fix_versions)

fix_versions = raw_input("Enter comma-separated fix version(s) [%s]: " % default_fix_versions)
fix_versions = input("Enter comma-separated fix version(s) [%s]: " % default_fix_versions)
if fix_versions == "":
fix_versions = default_fix_versions
fix_versions = fix_versions.replace(" ", "").split(",")
Expand All @@ -277,11 +301,12 @@ def get_version_json(version_str):

jira_fix_versions = map(lambda v: get_version_json(v), fix_versions)

resolve = filter(lambda a: a['name'] == "Resolve Issue", asf_jira.transitions(jira_id))[0]
resolve = filter(lambda a: a["name"] == "Resolve Issue", asf_jira.transitions(jira_id))[0]
asf_jira.transition_issue(
jira_id, resolve["id"], fixVersions=jira_fix_versions, comment=comment)
jira_id, resolve["id"], fixVersions=jira_fix_versions, comment=comment
)

print "Successfully resolved %s with fixVersions=%s!" % (jira_id, fix_versions)
print("Successfully resolved %s with fixVersions=%s!" % (jira_id, fix_versions))


def resolve_jira_issues(title, merge_branches, comment):
Expand All @@ -293,13 +318,13 @@ def resolve_jira_issues(title, merge_branches, comment):
resolve_jira_issue(merge_branches, comment, jira_id)


#branches = get_json("%s/branches" % GITHUB_API_BASE)
#branch_names = filter(lambda x: x.startswith("branch-"), [x['name'] for x in branches])
# branches = get_json("%s/branches" % GITHUB_API_BASE)
# branch_names = filter(lambda x: x.startswith("branch-"), [x['name'] for x in branches])
# Assumes branch names can be sorted lexicographically
#latest_branch = sorted(branch_names, reverse=True)[0]
# latest_branch = sorted(branch_names, reverse=True)[0]
latest_branch = "master"

pr_num = raw_input("Which pull request would you like to merge? (e.g. 23): ")
pr_num = input("Which pull request would you like to merge? (e.g. 23): ")
pr = get_json("%s/pulls/%s" % (GITHUB_API_BASE, pr_num))
pr_events = get_json("%s/issues/%s/events" % (GITHUB_API_BASE, pr_num))

Expand All @@ -313,39 +338,41 @@ def resolve_jira_issues(title, merge_branches, comment):

# Merged pull requests don't appear as merged in the GitHub API;
# Instead, they're closed by asfgit.
merge_commits = \
[e for e in pr_events if e["actor"]["login"] == "asfgit" and e["event"] == "closed"]
merge_commits = [e for e in pr_events if e["actor"]["login"] == "asfgit" and e["event"] == "closed"]

if merge_commits:
merge_hash = merge_commits[0]["commit_id"]
message = get_json("%s/commits/%s" % (GITHUB_API_BASE, merge_hash))["commit"]["message"]

print "Pull request %s has already been merged, assuming you want to backport" % pr_num
commit_is_downloaded = run_cmd(['git', 'rev-parse', '--quiet', '--verify',
"%s^{commit}" % merge_hash]).strip() != ""
print("Pull request %s has already been merged, assuming you want to backport" % pr_num)
commit_is_downloaded = (
run_cmd(["git", "rev-parse", "--quiet", "--verify", "%s^{commit}" % merge_hash]).strip()
!= ""
)
if not commit_is_downloaded:
fail("Couldn't find any merge commit for #%s, you may need to update HEAD." % pr_num)

print "Found commit %s:\n%s" % (merge_hash, message)
print("Found commit %s:\n%s" % (merge_hash, message))
cherry_pick(pr_num, merge_hash, latest_branch)
sys.exit(0)

if not bool(pr["mergeable"]):
msg = "Pull request %s is not mergeable in its current form.\n" % pr_num + \
"Continue? (experts only!)"
msg = (
"Pull request %s is not mergeable in its current form.\n" % pr_num
+ "Continue? (experts only!)"
)
continue_maybe(msg)

print ("\n=== Pull Request #%s ===" % pr_num)
print ("title\t%s\nsource\t%s\ntarget\t%s\nurl\t%s" % (
title, pr_repo_desc, target_ref, url))
print("\n=== Pull Request #%s ===" % pr_num)
print("title\t%s\nsource\t%s\ntarget\t%s\nurl\t%s" % (title, pr_repo_desc, target_ref, url))
continue_maybe("Proceed with merging pull request #%s?" % pr_num)

merged_refs = [target_ref]

merge_hash = merge_pr(pr_num, target_ref)

pick_prompt = "Would you like to pick %s into another branch?" % merge_hash
while raw_input("\n%s (y/n): " % pick_prompt).lower() == "y":
while input("\n%s (y/n): " % pick_prompt).lower() == "y":
merged_refs = merged_refs + [cherry_pick(pr_num, merge_hash, latest_branch)]

if JIRA_IMPORTED:
Expand All @@ -354,8 +381,8 @@ def resolve_jira_issues(title, merge_branches, comment):
jira_comment = "Issue resolved by pull request %s\n[%s/%s]" % (pr_num, GITHUB_BASE, pr_num)
resolve_jira_issues(title, merged_refs, jira_comment)
else:
print "JIRA_USERNAME and JIRA_PASSWORD not set"
print "Exiting without trying to close the associated JIRA."
print("JIRA_USERNAME and JIRA_PASSWORD not set")
print("Exiting without trying to close the associated JIRA.")
else:
print "Could not find jira library. Run 'sudo pip install jira' to install."
print "Exiting without trying to close the associated JIRA."
print("Could not find jira library. Run 'sudo pip install jira' to install.")
print("Exiting without trying to close the associated JIRA.")
9 changes: 5 additions & 4 deletions dev-support/database/init-database.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@

import mysql.connector

conn = mysql.connector.connect(
user='root', password='password', host='127.0.0.1')
conn = mysql.connector.connect(user="root", password="password", host="127.0.0.1")

cursor = conn.cursor(buffered=True)

Expand All @@ -35,10 +34,12 @@ def commit(sql):
# Commit your changes in the database
conn.commit()

except:
except mysql.connector.Error as err:
print("Something went wrong: {}".format(err))
# Rolling back in case of error
conn.rollback()


def commit_from_file(file_path):
with open(file_path) as f:
for result in cursor.execute(f.read(), multi=True):
Expand Down Expand Up @@ -76,4 +77,4 @@ def commit_from_file(file_path):
commit("GRANT ALL PRIVILEGES ON *.* TO 'metastore'@'%';")
commit("use metastore;")
commit_from_file("./dev-support/database/metastore.sql")
commit("show tables;")
commit("show tables;")
Loading

0 comments on commit 6fa91e3

Please sign in to comment.