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

handling issues 347,364 #385

Merged
merged 4 commits into from
Jul 13, 2023
Merged

handling issues 347,364 #385

merged 4 commits into from
Jul 13, 2023

Conversation

naraharip2017
Copy link
Collaborator

@naraharip2017 naraharip2017 commented Jul 12, 2023

Handling following issues

Testing done

  • Build plugin locally and ran unit tests. Verified changes on local running jenkins server

  • Verified the default asg when creating/update a ec2 fleet cloud is please-select.

  • Set default for asg to please-select and tried to save. Verified a failure occurs on saving the ec2 fleet cloud configuration

  • Created two ASGs, configured one for a ec2 fleet cloud, deleted it, and verified that the cloud uses please-select as the asg option and not the other ASG

Tasks

Preview Give feedback
No tasks being tracked yet.

@@ -31,7 +31,7 @@

<f:description>Fleet list will be available once region and credentials are specified. Only maintain supported, see help</f:description>
<f:entry title="${%EC2 Fleet}" field="fleet">
<f:select/>
<f:select checkMethod="post"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this required? Does it make any difference to the validation? 🤔

Copy link
Collaborator Author

@naraharip2017 naraharip2017 Jul 13, 2023

Choose a reason for hiding this comment

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

Previously was trying a different implementation, and was getting an error Jenkins: 403 No valid crumb was included in the request, which required making the check method a post request. But, should not need it for the current approach. Thanks, updating

Copy link
Collaborator

@pdk27 pdk27 left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants