[GitHub] [beam] mxm commented on pull request #12385: [BEAM-10527] Migrate Flink and Spark tests to pytest.

2020-10-01 Thread GitBox 


mxm commented on pull request #12385:
URL: https://github.com/apache/beam/pull/12385#issuecomment-702305517


   This looks good to me after rebasing. Thanks for porting this to pytest!



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [beam] mxm commented on pull request #12385: [BEAM-10527] Migrate Flink and Spark tests to pytest.

2020-08-11 Thread GitBox 


mxm commented on pull request #12385:
URL: https://github.com/apache/beam/pull/12385#issuecomment-672090815


   > > > I have already spent a long time trying to fix quotes, so I can't help 
but wondering: why do we need flinkCompatibilityMatrixPROCESS in the first 
place, when it is not being run anywhere? If it's important, shouldn't we add 
it to some postcommit?
   > > 
   > > 
   > > Another solution I had in mind was reworking the `--environment_config` 
option. JSON blobs are unwieldy, and overloading the `--environment_config` 
option is confusing to the user. We could make each field in the PROCESS 
`--environment_config` blob its own argument, and then reject these arguments 
when `environment_type != PROCESS`.
   > 
   > @mxm what do you think about 
https://issues.apache.org/jira/browse/BEAM-10671?
   
   I think it makes sense. Especially for error reporting by having dedicated 
argument parsers for all environments parameters.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [beam] mxm commented on pull request #12385: [BEAM-10527] Migrate Flink and Spark tests to pytest.

2020-08-03 Thread GitBox 


mxm commented on pull request #12385:
URL: https://github.com/apache/beam/pull/12385#issuecomment-667895488


   Thanks for fixing the quoting issue!
   
   > I got `flinkCompatibilityMatrixPROCESS` to pass on my machine by escaping 
the arguments via `${1@Q}`. Apparently whatever shell Jenkins is using does not 
support this. I will have to find a better solution.
   
   I think that feature only works in newer versions of `bash`. 
   
   > why do we need `flinkCompatibilityMatrixPROCESS` in the first place, when 
it is not being run anywhere? If it's important, shouldn't we add it to some 
postcommit?
   
   `flinkCompatibilityMatrixPROCESS` was how we ran the PVR tests to avoid a 
dependency on the container build (for speed). I was under the assumption that 
we are still doing that. That probably changed when we added the external 
transform tests which require the Java container. I agree that we should at 
least have a post commit.



This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org