Skip to content
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

eng, revert tsp-client in sdk automation #38983

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 7 additions & 30 deletions eng/mgmt/automation/generate_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,40 +47,17 @@ def sdk_automation_typespec(config: dict) -> List[dict]:
service = None
module = None
try:
def remove_prefix(text, prefix):
if text.startswith(prefix):
return text[len(prefix):]
return text

def find_sdk_folder():
cmd = ['git', 'add', '.']
check_call(cmd, sdk_root)

cmd = ['git', 'status', '--porcelain', '**/tsp-location.yaml']
logging.info('Command line: ' + ' '.join(cmd))
output = subprocess.check_output(cmd, cwd=sdk_root)
output_str = str(output, 'utf-8')
git_items = output_str.splitlines()
sdk_folder = None
if len(git_items) > 0:
tsp_location_item: str = git_items[0]
sdk_folder = tsp_location_item[1:].strip()[0:-len('/tsp-location.yaml')]

cmd = ['git', 'reset', '.']
check_call(cmd, sdk_root)

return sdk_folder

repo = remove_prefix(repo_url, 'https://github.com/')
cmd = ['npx', 'tsp-client', 'init', '--debug', '--tsp-config', tsp_dir, '--commit', head_sha, '--repo', repo]
check_call(cmd, sdk_root)

sdk_folder = find_sdk_folder()
cmd = ['pwsh', './eng/common/scripts/TypeSpec-Project-Process.ps1', tsp_dir, head_sha, repo_url]
Copy link
Member

Choose a reason for hiding this comment

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

@weidongxu-microsoft why did this need to be reverted? Do we have a plan to switch back?

Copy link
Member Author

@weidongxu-microsoft weidongxu-microsoft Feb 29, 2024

Choose a reason for hiding this comment

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

It was due to the failure when the automation runs from a private repo.

PR https://github.com/Azure/azure-rest-api-specs-pr/pull/16496/checks?check_run_id=21889059048

failure
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3520093&view=logs&j=a8a7a537-82b0-583c-7971-bac70b9822ca&s=eb9754a7-3885-5b5b-bd91-16c95dd7881e&t=37e3947b-3cfb-5d36-86ba-0e22bb7dbc33&l=274

automation passes after this revert PR
https://dev.azure.com/azure-sdk/internal/_build/results?buildId=3520093&view=logs&j=a8a7a537-82b0-583c-7971-bac70b9822ca&t=37e3947b-3cfb-5d36-86ba-0e22bb7dbc33&s=eb9754a7-3885-5b5b-bd91-16c95dd7881e

I didn't have time to investigate why it fails (and it's pretty hard to investigate on the pr repo), but the ask is pretty urgent (and Java is the only automation that uses tsp-client). Therefore, I did the revert PR first to unblock service.

We can switch back when we figure out why it fails in private repo.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @weidongxu-microsoft for the context.

@catalinaperalta based on Azure/azure-sdk-tools#7292 I thought we supported the private repo. Is there a bug or more work to be done to enable it?

Copy link
Member Author

Choose a reason for hiding this comment

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

issue on azure-sdk-tools Azure/azure-sdk-tools#8012

Copy link
Member

Choose a reason for hiding this comment

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

Will take a look, I missed the original notification on this thread. Thanks for opening an issue!

Copy link
Member

@catalinaperalta catalinaperalta Apr 5, 2024

Choose a reason for hiding this comment

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

IIRC in the pipeline we'd need to have git credentials configured for the git sparse checkout command to work. A few months ago I spoke to Ben about this since I'd seen this issue Azure/azure-sdk-tools#3222 opened about this exact scenario. I think we'd need to have a PAT or git credentials configured in the pipeline for this to work. Locally for private spec repo support we rely on folks already being logged in with git/having their credentials configured. If we want to use a PAT in the pipeline...then I think there would be additional work to do in tsp-client. Otherwise if we can configure credentials in the pipeline like it's mentioned in this comment, I believe tsp-client should work as is.

logging.info('Command line: ' + ' '.join(cmd))
output = subprocess.check_output(cmd, cwd=sdk_root)
output_str = str(output, 'utf-8')
script_return = output_str.splitlines()[-1] # the path to sdk folder
sdk_folder = os.path.relpath(script_return, sdk_root)
logging.info('SDK folder: ' + sdk_folder)
if sdk_folder:
succeeded = True
except subprocess.CalledProcessError as error:
logging.error(f'tsp-client init fail: {error}')
logging.error(f'TypeSpec-Project-Process.ps1 fail: {error}')

if succeeded:
# check require_sdk_integration
Expand Down
3 changes: 0 additions & 3 deletions eng/mgmt/automation/init.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ export NVM_DIR="$HOME/.nvm"
nvm install v18.15.0
nvm alias default node

# install tsp-client globally (local install may interfere with tooling)
npm install -g @azure-tools/typespec-client-generator-cli

cat << EOF > $2
{"envs": {"PATH": "$JAVA_HOME_11_X64/bin:$PATH", "JAVA_HOME": "$JAVA_HOME_11_X64"}}
EOF