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.

Sign up for GitHub

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

Jump to bottom

Changing the commit message to 1000 characters #296

Closed
wants to merge 2 commits into from
Closed

Changing the commit message to 1000 characters #296

wants to merge 2 commits into from

Conversation

shpraka
Copy link
Contributor

@shpraka shpraka commented Feb 15, 2023 •
edited
Loading

This covers #288.

Recently we found out that 7000 doesn't work in some scenarios intermittently, keeping it a bit lower for the deployment to go through.

@shpraka shpraka temporarily deployed to automation test February 15, 2023 07:00 — with GitHub Actions Inactive
@shpraka shpraka temporarily deployed to automation test February 15, 2023 07:00 — with GitHub Actions Inactive
kumaraksh1
kumaraksh1 approved these changes Feb 17, 2023
View reviewed changes
Copy link
Collaborator

@kumaraksh1 kumaraksh1 left a comment

Choose a reason for hiding this comment

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

LGTM!!

@github-actions
Copy link

github-actions bot commented Mar 3, 2023

This PR is idle because it has been open for 14 days with no activity.

@github-actions github-actions bot added the idle label Mar 3, 2023
@AaronL1011
Copy link

Any plans to merge this one? This issue is intermittently blocking our pipelines, would love to see progress 🙏

@github-actions github-actions bot removed the idle label Mar 6, 2023
@shpraka
Copy link
Contributor Author

shpraka commented Mar 6, 2023

@AaronL1011 Could you share the debug logs of the failed runs? I had tried reproducing same last week and it wasn't failing for me with existing limit?

@Methuselah96
Copy link
Contributor

Added an example of it failing in #288.

@AaronL1011
Copy link

AaronL1011 commented Mar 7, 2023 •
edited
Loading

@AaronL1011 Could you share the debug logs of the failed runs? I had tried reproducing same last week and it wasn't failing for me with existing limit?

Unfortunately its all under private repos, and I can't share the entire action run, however it's virtually the same as the failing action @Methuselah96 has shared. Ive attached as much of the log as possible, the zipdeploy URL was ~2159 chars long.

##[debug][POST] *** { 2159 character-long URL full of commit messages } ***
##[debug]Could not parse response body.
##[debug]***
##[debug]ZIP Deploy response: "statusCode":404,"statusMessage":"Not Found","headers":"content-length":"103","connection":"close","content-type":"text/html","date":"Mon, 06 Mar 2023 23:54:29 GMT","server":"Microsoft-IIS/10.0","x-powered-by":"ASP.NET","body":"The resource you are looking for has been removed, had its name changed, or is temporarily unavailable."
Error: Failed to deploy web package to App Service.
Error: Deployment Failed with Error: Error: Failed to deploy web package to App Service.
Not Found (CODE: 404)

AaronL1011
AaronL1011 reviewed Mar 7, 2023
View reviewed changes
lib/actionparameters.js Outdated
@@ -37,7 +37,7 @@ class ActionParameters {
/**
* Trimming the commit message because it is used as a param in uri of deployment api. And sometimes, it exceeds the max length of http URI.
*/
this._commitMessage = github.context.eventName === 'push' ? github.context.payload.head_commit.message.slice(0, 7000) : "";
this._commitMessage = github.context.eventName === 'push' ? github.context.payload.head_commit.message.slice(0, 2000) : "";
Copy link

Choose a reason for hiding this comment

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

Given @IGx89's point in issue #288 on URL encoding and how it will largely increase the length of the zipdeploy URL post-encode, should this trim be closer to 1000? Ditto for the other file change 👍

@shpraka shpraka temporarily deployed to automation test March 9, 2023 10:45 — with GitHub Actions Inactive
@shpraka shpraka changed the title Changing the commit message to 2000 characters Changing the commit message to 1000 characters Mar 9, 2023
kumaraksh1
kumaraksh1 approved these changes Mar 9, 2023
View reviewed changes
Copy link
Collaborator

@kumaraksh1 kumaraksh1 left a comment

Choose a reason for hiding this comment

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

LGTM !!

@github-actions
Copy link

This PR is idle because it has been open for 14 days with no activity.

@github-actions github-actions bot added the idle label Mar 23, 2023
@IGx89
Copy link

IGx89 commented Mar 23, 2023

This looks good to me too, and appears like it should resolve #288. Still waiting on something before merging?

@github-actions github-actions bot removed the idle label Mar 23, 2023
@AaronL1011
Copy link

Yeah keen to get this one on the move, is there a hold up in the deploy pipeline process? Not sure who is the right person to push this through.

kumaraksh1
kumaraksh1 approved these changes Mar 24, 2023
View reviewed changes
Copy link
Collaborator

@kumaraksh1 kumaraksh1 left a comment

Choose a reason for hiding this comment

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

LGTM!!

kumaraksh1
kumaraksh1 approved these changes Mar 24, 2023
View reviewed changes
Copy link
Collaborator

@kumaraksh1 kumaraksh1 left a comment

Choose a reason for hiding this comment

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

LGTM!!

shubham805
shubham805 approved these changes Mar 24, 2023
View reviewed changes
Copy link
Contributor

@shubham805 shubham805 left a comment

Choose a reason for hiding this comment

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

LGTM

@shpraka
Copy link
Contributor Author

shpraka commented Mar 27, 2023

This PR is covered as part of #301.

@shpraka shpraka closed this Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AaronL1011 AaronL1011 AaronL1011 left review comments

@kumaraksh1 kumaraksh1 kumaraksh1 approved these changes

@shubham805 shubham805 shubham805 approved these changes

@kaverma kaverma Awaiting requested review from kaverma

@kanika1894 kanika1894 Awaiting requested review from kanika1894

@BALAGA-GAYATRI BALAGA-GAYATRI Awaiting requested review from BALAGA-GAYATRI

@surenderssm surenderssm Awaiting requested review from surenderssm surenderssm is a code owner

@kumaramit-msft kumaramit-msft Awaiting requested review from kumaramit-msft

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@shpraka @AaronL1011 @Methuselah96 @IGx89 @shubham805 @kumaraksh1