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

Set DB environment for java+ as well as for java #6248

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

jhorvath
Copy link
Contributor

To correctly set environment variables for database connection we need to register DBConfigurationProvider for both targets - java+ and java.

@jhorvath jhorvath added LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests labels Jul 24, 2023
@jhorvath jhorvath requested a review from sdedic July 24, 2023 16:52
@jhorvath jhorvath changed the base branch from master to delivery July 25, 2023 07:17
@jhorvath jhorvath added this to the NB19 milestone Jul 25, 2023
Copy link
Member

@sdedic sdedic left a comment

Choose a reason for hiding this comment

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

I would suggest to make 2 instances of DBConfigurationProvider instead of using a global variable -- but it seems a matter of style here: none of the reloadable NBLS-related state is used.

@jhorvath jhorvath merged commit 9032ecc into apache:delivery Jul 25, 2023
@jhorvath jhorvath deleted the db-env branch July 25, 2023 11:07
@neilcsmith-net
Copy link
Member

@jhorvath only the release team should be merging to delivery.

@neilcsmith-net
Copy link
Member

cc/ @mbien out of interest, when you were setting up branch protection, did you notice whether asf.yml exposed the ability to restrict committing on certain branches to certain accounts?

@MartinBalin
Copy link
Contributor

@jhorvath only the release team should be merging to delivery.

@neilcsmith-net sorry, I've suggested to Jan he should merge to delivery.

@neilcsmith-net
Copy link
Member

@MartinBalin no worries - you're on it anyway. I'll be syncing for rc2 tomorrow morning, so unless there's anything urgent let's wait for both sync PRs before merging anything else.

@mbien
Copy link
Member

mbien commented Jul 25, 2023

cc/ @mbien out of interest, when you were setting up branch protection, did you notice whether asf.yml exposed the ability to restrict committing on certain branches to certain accounts?

@neilcsmith-net I don't think this is possible. The doc at least does not list this as an option.

edit: gh itself seems to be supporting it: https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/managing-protected-branches/about-protected-branches#restrict-who-can-push-to-matching-branches

going to ask on infra slack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSP [ci] enable Language Server Protocol tests VSCode Extension [ci] enable VSCode Extension tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants