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

config.json cleanup #5866

Merged
merged 7 commits into from
Mar 9, 2020
Merged

config.json cleanup #5866

merged 7 commits into from
Mar 9, 2020

Conversation

ajeetj
Copy link
Contributor

@ajeetj ajeetj commented Feb 27, 2020

Migrated check_make_parser, check_make_visitor, java_test to github actions.
Removed python dependency from bootstrap.sh.

Signed-off-by: Ajeet jain [email protected]

@ajeetj ajeetj requested a review from sougou as a code owner February 27, 2020 06:28
@arindamnayak arindamnayak force-pushed the config_cleanup branch 3 times, most recently from 1401fdc to 1be35f6 Compare March 4, 2020 08:11
@deepthi deepthi changed the title [DO NOT MEREG- TESTING] config.json cleanup config.json cleanup Mar 4, 2020
run: |
make minimaltools

- name: check_make_parser
Copy link
Member

Choose a reason for hiding this comment

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

is this supposed to be check_make_visitor?

Copy link
Contributor

Choose a reason for hiding this comment

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

done.

@@ -1,69 +1,25 @@
{
"Tests": {
"check_make_parser": {
Copy link
Member

Choose a reason for hiding this comment

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

based on offline discussion, I think we should add the two check tests back.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since those are added to github actions, I removed it from here. But I have to keep the java test here, as it needs docker dependency and this is possible via go run test.go java . Whereas these 2 checks/tests does not need any docker dependency.

ajeetj and others added 4 commits March 5, 2020 12:16
Signed-off-by: Ajeet jain <[email protected]>
Signed-off-by: Arindam Nayak <[email protected]>
Signed-off-by: Arindam Nayak <[email protected]>
Signed-off-by: Arindam Nayak <[email protected]>
@arindamnayak arindamnayak force-pushed the config_cleanup branch 2 times, most recently from 200ba90 to 6b3a24d Compare March 5, 2020 07:12
@arindamnayak arindamnayak force-pushed the config_cleanup branch 3 times, most recently from 43edb19 to af1058f Compare March 5, 2020 07:48
@arindamnayak
Copy link
Contributor

Do we need python vtgategrpc client test ? For now, I have added python GRPC library back to bootstrap.sh as python client testing (mainly vtgaterpc client test from python) needs the GRPC library.

.github/workflows/cluster_endtoend.yml Show resolved Hide resolved
Comment on lines +41 to +43
export VTROOT=/<vitess path>/vitess
export VTDATAROOT=${VTROOT}/vtdataroot
export PATH=${VTROOT}/bin:${PATH}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this strictly required? build.env should be able to guess these. In future we may even be able to deprecate VTROOT as it can be implied.

Copy link
Contributor

Choose a reason for hiding this comment

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

For now, it needs vitess binaries to be in PATH and we should also have VTDATAROOT . So when VTROOT deprecates, we can change this .

test/config.json Show resolved Hide resolved
@morgo
Copy link
Contributor

morgo commented Mar 5, 2020

Do we need python vtgategrpc client test ? For now, I have added python GRPC library back to bootstrap.sh as python client testing (mainly vtgaterpc client test from python) needs the GRPC library.

I think the answer is no. Let's remove it since it is really Python 2.7, which is no longer supported.

Signed-off-by: Arindam Nayak <[email protected]>
@arindamnayak
Copy link
Contributor

Do we need python vtgategrpc client test ? For now, I have added python GRPC library back to bootstrap.sh as python client testing (mainly vtgaterpc client test from python) needs the GRPC library.

I think the answer is no. Let's remove it since it is really Python 2.7, which is no longer supported.

Now, I have removed it and also changed the shard number of individually runnable test to -1.

@morgo morgo self-requested a review March 9, 2020 15:02
@morgo morgo merged commit 73ceb9c into vitessio:master Mar 9, 2020
@arindamnayak arindamnayak deleted the config_cleanup branch March 9, 2020 17:37
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.

4 participants