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

feat(datahub-upgrade): adding concurrency policy for the cronjobs #376

Merged
merged 4 commits into from
Oct 12, 2023

Conversation

dheerajrampally
Copy link
Contributor

@dheerajrampally dheerajrampally commented Oct 6, 2023

Context

By default, the concurrency policy for the cronjobs is Allow. In a few deployment strategies, concurrencyPolicy is defaulted to Forbid. In those scenarios, the upstream chart should have the option for it to be set.

This PR will help in accomplishing that.

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable)

@dheerajrampally dheerajrampally changed the title adding concurrency policy for the cronjobs feat(datahub-upgrade): adding concurrency policy for the cronjobs Oct 6, 2023
@upendra-vedullapalli
Copy link
Contributor

I though you might have missed it here but it was taken care already.

  • Good if you can update corresponding README files and chart versions.

@dheerajrampally
Copy link
Contributor Author

@upendra-vedullapalli I can update. Let me take a look.

@dheerajrampally
Copy link
Contributor Author

@upendra-vedullapalli I see that there is no dependency chart in datahub/datahub/Chart.yaml for the datahub upgrade job where I can bump the version. So I presume, it is not required. Please let me know if its not the case.

Copy link
Contributor

@upendrao upendrao left a comment

Choose a reason for hiding this comment

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

Please a take a look at my comments.

Copy link
Contributor

@upendrao upendrao left a comment

Choose a reason for hiding this comment

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

LGTM. Probably you need a maintainers approval for merge 🤞

@dheerajrampally
Copy link
Contributor Author

LGTM. Probably you need a maintainers approval for merge 🤞

How do I get that privilege?

@RyanHolstien RyanHolstien merged commit 4a19805 into acryldata:master Oct 12, 2023
1 check passed
@RyanHolstien
Copy link
Contributor

Thanks for the contribution! 😄

@maggiehays maggiehays added the hacktoberfest-accepted Acceptance for hacktoberfest https://hacktoberfest.com/participation/ label Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Acceptance for hacktoberfest https://hacktoberfest.com/participation/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants