[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-23 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-590052010
 
 
   Ah, I love the computer history museum! I’m actually a remote worker to 
Stanford based out of Colorado, so there is still an hour or so drive, few hour 
flight, and expensive Bay Area hotels in the way of me participating. Maybe in 
the future if there is an option to join remotely or get reimbursed, I’d be 
able to come. And in the meantime I can definitely look over content and give 
feedback, if that might help.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-23 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-590050857
 
 
   Thanks everyone :) It's just a little before 3:00am here but I still think 
I'm going to have a tiny dance party before I slip back to sleep. :tada: 
   
   Really looking forward to someone using this in the wild!


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-23 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-590049375
 
 
   holy crap it passed! Woot!!


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-22 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-590029643
 
 
   Alright, rebase is just done! I think it's likely something transient, 
because the rebase just had changes to UPDATING.md (note that right before the 
set of last pushes I rebased when there was a conflict with setup.py). So let's 
hope that the transient error doesn't show up again!


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-22 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-590012811
 
 
   @potiuk for the recent tests, I'm seeing a redirect  (302) for an address 
that (used to be?) 200? I don't see any singularity operator fails to debug so 
I don't have any new commits to push to re-test. Is it possible for someone on 
your team to trigger a re-run, in the case of a one time spoof, or to take a 
look into the redirects? Here is the output that I'm referencing:
   
   ```
   tests/www/test_views.py:98: in check_content_in_response
   
   self.assertEqual(resp_code, resp.status_code)
   
   E   AssertionError: 200 != 302
   ```


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-20 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-589395121
 
 
   looks like the error linked above was one off - a later build didn't trigger 
it.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-20 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-589285097
 
 
   Quick question - I don't see any reference to the singularity operator 
[here](https://travis-ci.org/apache/airflow/jobs/653112190) but the tests 
failed - do you know what's up? I'll focus on just the vanilla postgres for 
now, that seems to have errors for the operator.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-19 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-588482545
 
 
   Hmm, so I definitely added the disable line, but it doesn't seem to be 
taking:
   
   
https://github.com/apache/airflow/pull/7191/commits/cef6c077927e996f5c910c872a775850a25596f0
   
   It's triggering for line 65, so I suppose I can try adding it there...


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-19 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-588463321
 
 
   @potiuk I tried this huge build multiple times before, and they all resulted 
in errors. I'm glad to try it again, but I've had much better luck with reading 
the errors and addressing the errors verbatim.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-19 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-588448150
 
 
   okay, I found plenty of examples and copied those, so I didn't need to run 
it manually. Let's hope that works!


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-19 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-588442023
 
 
   One quick note about the docs for pylint - the files are now changed to have 
pylint_main.sh and pylint_tests.sh. Also, this unexpectedly started that huge 
build process that failed before. Given a .pylintrc file in some root and 
running pylint, it would be great to see a much simpler (not wrapped / 
abstracted) run of pylint on a single file.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-19 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-588437467
 
 
   okay, I'll try that! It does seem like I'm between a rock and a hard place, 
glad that you are okay with disabling pylint here.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-19 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-588357686
 
 
   How would you like me to address this?
   
   ```
   
PATH=/root:/usr/local/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/opt/hive/bin:/usr/local/go/bin:/opt/gcloud/bin
   
   * Module airflow.providers.singularity.operators.singularity
   
   airflow/providers/singularity/operators/singularity.py:65:4: R0913: Too many 
arguments (12/10) (too-many-arguments)
   ```
   I had added working_dir to kwargs, but then during the test there was an 
error that the argument wasn't found. So I added it, but now I'm over.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-18 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-587936273
 
 
   Actually it looks like it wasn't happy with working_dir as a kwargs, I'll 
try adding it to the type definitions to see if that changes anything...


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-18 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-587935001
 
 
   Can somebody explain what the task_id (provided as task-id?) is expected to 
work, be parsed, etc? @mik-laj added it to the test cases last year, and it's 
not clear if I need to be parsing this for the operator - just trying as it was 
it failed that the key word argument wasn't valid.
   
   >airflow.exceptions.AirflowException: Invalid arguments were 
passed to SingularityOperator (task_id: task-id). Invalid arguments were:


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-18 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-587823490
 
 
   docs build passed! Tiny progress :) 


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-17 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-587082403
 
 
   Ah thank you! Just added.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-15 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586651037
 
 
   Can you show me where to add in the toc tree? I thought I grepped for 
similar providers and added it, but it's not happy:
   
   > opt/airflow/docs/_api/example_singularity_operator/index.rst: WARNING: 
document isn't included in any toctree


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-15 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586644781
 
 
   > Sorry - devl...@airflow.apache.org - see CONTRIB.rst. I was typing it on 
my tablet and autocomplete kicked in.
   
   Haha, no worries :) I was thinking it was some kind of special club that I 
am far too old and uncool for. Which is okay too! :)


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-15 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586640931
 
 
   Okay the changes I've just made:
   
-  moved `airflow/contrib/operators/singularity_operator.py` to 
`airflow/providers/singularity/operators/__init__.py` and added `__init__.py` 
in folder above to make module.
- updated docs to point to  - 
:mod:`airflow.providers.operators.singularity`
- added `airflow/providers/singularity/operators/index` to the 
autoapi_templates index file (I just figured this out with grep, I had already 
triggered the new testing with fixes to setup.py
- for the mock tests, I have no idea how to do this, but I changed the 
image / commands to actual commands / images that might be used. As for the 
rest, no clue.
   
   I don't know what devliscie is, just fyi.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-15 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586639114
 
 
   Conflict resolved - it was the same line as before, another package was 
added! I added a new line (it was getting too long) so hopefully that won't 
make the linter unhappy...


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-15 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586638581
 
 
   Note that there were conflicts with setup.py before my set of last commits, 
and I resolved them here.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-15 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586638274
 
 
   I can try to address the conflicts, but I can't rebase it seems:
   
   ```
   Auto-merging setup.py
   CONFLICT (content): Merge conflict in setup.py
   Auto-merging scripts/ci/in_container/entrypoint_ci.sh
   Auto-merging docs/operators-and-hooks-ref.rst
   Auto-merging Dockerfile
   error: could not apply bacc3b54c... adding singularity operator and tests
   Resolve all conflicts manually, mark them as resolved with
   "git add/rm ", then run "git rebase --continue".
   You can instead skip this commit: run "git rebase --skip".
   To abort and get back to the state before "git rebase", run "git rebase 
--abort".
   Could not apply bacc3b54c... adding singularity operator and tests
   ```


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-14 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586424350
 
 
   Questions:
   
- where does example_singularity_operator.py go - it used to be under 
contrib/example_dags but now, that entire folder is empty?
- this error is triggering for the docs (is it related to the above)? I 
don't see any script_files with grep, nor do I see how super() relates to the 
example_singularity_operator.py file.
   ```
   writing additional pages...  
search/usr/local/lib/python3.6/site-packages/sphinx_rtd_theme/search.html:20: 
RemovedInSphinx30Warning: To modify script_files in the theme is deprecated. 
Please insert a 

[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-14 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586178588
 
 
   I totally agree on all points! I can’t imagine what managing a PR board like 
this must be like.
   
   I’m mostly happy that the pre-checks passed and we are inching forward! I’ll 
be able to look at this again later today.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-13 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586067049
 
 
   High level feedback since this was a topic earlier:
- the extra linting for the Dockerfile, which comes down to fairly trivial 
things like using cd in a command for one layer and using curl instead of wget, 
has led to several additional errors that are frustrating to deal with. If I 
knew the project perfectly sure I might have known these in advance, but for a 
new contributor that gets these bugs (which are somewhat subjective) that then 
has to figure them out and fix, it's an extra pain that simply doesn't need to 
exist.
- given the long duration of a PR, the addition of new linters / tests 
makes it even more confusing.
   
   I'm happy to keep providing this feedback, and if you don't want it let me 
know!


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-13 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586039637
 
 
   okay I'm going to assume that this "no pragma" is new and just remove the 
lines from the top of those files. I also used the docker linting container 
locally with the linting configuration and container, and it seems to produce 
no output after some fixes so I'm hoping this works. I don't necessarily think 
the changes are better - it didn't let me use cd in a multi line statement, and 
instead I had to change to artificially add a WORKDIR between, so now there are 
two layers instead of just one. But I guess the linting passes, lol!


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-13 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586036255
 
 
   What are these new errors about encoding  / pragma? I literally didn't 
change any of these files
   ```
   
pragma.Failed
   
   - hook id: fix-encoding-pragma
   
   - duration: 0.14s
   
   - exit code: 1
   
   - files were modified by this hook
   
   Removed encoding pragma from 
airflow/contrib/operators/singularity_operator.py
   
   Removed encoding pragma from 
airflow/contrib/example_dags/example_singularity_operator.py
   
   Removed encoding pragma from 
tests/contrib/operators/test_singularity_operator.py
   ```
   The Dockerlint errors are a bit much, but I'll jump through the hoops :/


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-13 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-586021498
 
 
   okay to so to update everyone following (and future me reading back to 
remember this) we are first trying a simple approach to install Singularity in 
the base container, and then the tests will run for all the various 
environments without a special runtime. The reason is because Singularity is 
not a service (like Docker has a daemon) but rather just a binary that can 
create instances / containers / otherwise. This first test will just be to see 
if singularity is detected in the host container when the executors run, 
granted that the resolved merge for setup.py didn't result in linting errors or 
what not.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-13 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-585967157
 
 
   Perfecto! I'll give this a shot.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-13 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-585946960
 
 
   > I assume we can start singularity from the host and be able to forward 
this connection to inside the airlfow-testing container so that we can connect 
to it. With Kind - we are starting it from within the container (by forwarded 
docker socket) , but it could be started from the host as well (mongo, kerberos 
and others are started from the host using docker-compose configuration and 
then we can connect to them from the "airflow-testing" by specifying their 
service names (mongo/kerberos etc).
   
   Ah yes this is what I wanted to ask about, specifically:
   
   > I assume we can start singularity from the host and be able to forward 
this connection to inside the airlfow-testing container so that we can connect 
to it. 
   
   Singularity is different from docker - it doesn't have a service or daemon, 
it's akin to an executable. It would be installed inside a container, and it 
wouldn't work to "start on the host and forward." 
   
   > started from the host using docker-compose configuration and then we can 
connect to them from the "airflow-testing" by specifying their service names 
(mongo/kerberos etc).
   
   You mean to say that we start a container with Singularity installed via 
docker-compose, and then run as a service? I think it would work to run things 
internally, I'm not sure the extent to which you could issue interactions from 
the outside. This I think is something that we could do:
   
1. installing Singularity as a binary inside a container (these bases 
already exist)
2. running the container via docker-compose
3. But then running tests inside of that container
   
   Is this possible?


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-02-13 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-585869248
 
 
   ping...


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-29 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-579881380
 
 
   okay just to clarify - you want a Singularity + Airflow container run via a 
similar kind cluster? You said something about adding specific commands 
`--start-singularity` and `--stop-singularity` and I'm not sure what / where 
that is referring to, and how / why we would want to start or stop singularity 
(it's not a service, it's just a binary installed).


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-29 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-579878775
 
 
   Oh wait I see, the "deploy_airflow_to_kubernetes.sh" detects the runtime and 
triggers the build to happen via: 
https://github.com/apache/airflow/blob/master/scripts/ci/in_container/kubernetes/docker/rebuild_airflow_image.sh.
 Let me see if I can mirror that with Singularity. 


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-29 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-579877224
 
 
   okay so I'm tracing the kubernetes (runtime) as an example, and I have a 
quick question. In scripts/ci/in_container/entrypoint_ci.sh I see that given 
the kubernetes runtime, you are setting some variable for a container name to 
be the airflow name with suffix "-kubernetes."
   
   ```bash
   if [[ ${RUNTIME:=""} == "kubernetes" ]]; then
   unset KRB5_CONFIG
   unset KRB5_KTNAME
   export AIRFLOW_KUBERNETES_IMAGE=${AIRFLOW_CI_IMAGE}-kubernetes
   AIRFLOW_KUBERNETES_IMAGE_NAME=$(echo "${AIRFLOW_KUBERNETES_IMAGE}" | cut 
-f 1 -d ":")
   export AIRFLOW_KUBERNETES_IMAGE_NAME
   AIRFLOW_KUBERNETES_IMAGE_TAG=$(echo "${AIRFLOW_KUBERNETES_IMAGE}" | cut 
-f 2 -d ":")
   export AIRFLOW_KUBERNETES_IMAGE_TAG
   fi
   ```
   What isn't clear is where this container is built, and then where it gets 
used again? At this point we are already sitting inside the airflow-testing 
container after a command like this:
   
   ```
   else if [[ ${RUNTIME:=} == "singularity" ]]; then
   set +u
   # shellcheck disable=SC2016
   docker-compose --log-level INFO \
 -f "${MY_DIR}/docker-compose/base.yml" \
 -f "${MY_DIR}/docker-compose/backend-${BACKEND}.yml" \
 -f "${MY_DIR}/docker-compose/runtime-singularity.yml" \
 "${INTEGRATIONS[@]}" \
 "${DOCKER_COMPOSE_LOCAL[@]}" \
run airflow-testing \
  '/opt/airflow/scripts/ci/in_container/entrypoint_ci.sh "${@}"' \
  /opt/airflow/scripts/ci/in_container/entrypoint_ci.sh "${@}"
# Note the command is there twice (!) because it is passed via bash 
-c
# and bash -c starts passing parameters from $0. TODO: fixme
   set -u
   ```
   and this would suggest the container is brought up inside of this container? 
Wouldn't it make more sense to have a docker-compose run that runs some 
equivalent of airflow-testing but with Singularity installed inside? 


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-28 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-579359117
 
 
   I think the third approach is best:
   
   > If you need something pre-backed in the Docker image itself then I think 
we need a separate Dockerfile with FROM: singularity and some steps to build 
the dockerfile before docker-compose is run (and refer to the newly built image 
in Docker compose. My best bet if that is the case, that would be that you 
build and test such images locally for local testing and once it works I can 
help to integrate it in the CI scripts properly (You can try yourself - this 
will likely be just appropriate 'docker build' command executed only when 
--integration singularity is used)
   
   because Singularity requires a fairly specific set of custom dependencies, 
and then takes a while to compile. I can generate this file locally and use the 
already existing bases on quay.io. Could you give me some pointers about where 
to write the Dockerfile, what example from the current codebase (link) is what 
I should try to copy the logic for?


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-22 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-577373099
 
 
   okay, I've taken a look at the docker-compose approach. Specifically, I've 
created a `integration-singularity.yml` file that uses a container base that 
provides Singularity (v3.5.1).
   
   ```yaml
   version: "2.2"
   services:
 singularity:
   image: quay.io/singularity/singularity:v3.5.1
   command: -F /etc/alpine-release
   entrypoint: /usr/bin/tail
   privileged: true
   volumes:
 - /dev/urandom:/dev/random   # Required to get non-blocking entropy 
source
   ```
   Note that I'm using privileged, which likely requires the sudo:true 
parameter for the testing setup. The traditional entrypoint is the Singularity 
executable, so I have a basic entrypoint/command to just keep the container 
running (and allow me to shell inside). Even outside of testing (which is 
fairly complicated and I'm hoping to not need to reproduce it locally) we can 
test if the basic interaction with Singularity would work inside the container.
   
   ```bash
   docker-compose --log-level INFO -f 
"scripts/ci/docker-compose/integration-singularity.yml" up -d
   ```
   
   Check that everything went up ok...
   
   ```bash
   docker-compose --log-level INFO -f 
"scripts/ci/docker-compose/integration-singularity.yml" logs
   Attaching to docker-compose_singularity_1_7d898ecbdbb0
   singularity_1_7d898ecbdbb0 | 3.10.3
   ```
   And that we continued running...
   
   ```bash
   $ docker-compose --log-level INFO -f 
"scripts/ci/docker-compose/integration-singularity.yml" ps
 Name Command   
State   Ports
   
--
   docker-compose_singularity_1_7d898ecbdbb0   /usr/bin/tail -F /etc/alpi ...   
Up  
   ```
   Great! And then we can shell inside. We confirm the version of singularity.
   
   ```bash
   $ docker exec -it 0081d687eeb1 bash
   bash-5.0# which singularity
   /usr/local/bin/singularity
   bash-5.0# singularity --version
   singularity version 3.5.1
   ```
   The basic thing we would need to do is start instances. We can actually 
build the SIF binary successfully, and let's do that first, pulling the 
container:
   
   ```bash
   $ docker exec -it 3e73d2d5b41e bash
   bash-5.0# which singularity
   /usr/local/bin/singularity
   bash-5.0# singularity pull docker://busybox
   INFO:Converting OCI blobs to SIF format
   INFO:Starting build...
   Getting image source signatures
   Copying blob bdbbaa22dec6 done
   Copying config b075e856be done
   Writing manifest to image destination
   Storing signatures
   2020/01/22 20:24:24  info unpack layer: 
sha256:bdbbaa22dec6b7fe23106d2c1b1f43d9598cd8fc33706cc27c1d938ecd5bffc7
   INFO:Creating SIF file...
   INFO:Build complete: busybox_latest.sif
   bash-5.0# 
   ```
   Now let's start the instance.
   
   ```bash
   bash-5.0# singularity instance start busybox_latest.sif busybox
   ERROR:   container cleanup failed: no instance found with name busybox
   FATAL:   container creation failed: mount /etc/localtime->/etc/localtime 
error: while mounting /etc/localtime: mount source /etc/localtime doesn't exist
   
   FATAL:   failed to start instance: while running 
/usr/local/libexec/singularity/bin/starter: exit status 255
   ```
   
   okay, let's trace back and add this to our docker-compose yml file, we will 
mount as a volume from the host and redo the above.
   
   ```yaml
   version: "2.2"
   services:
 singularity:
   image: quay.io/singularity/singularity:v3.5.1
   command: -F /etc/alpine-release
   entrypoint: /usr/bin/tail
   privileged: true
   volumes:
 - /etc/localtime:/etc/localtime
   ```
   
   Zooming ahead, let's recreate and then start the instance:
   
   ```bash
   bash-5.0# singularity instance start busybox_latest.sif busybox
   INFO:instance started successfully
   ```
   Success! 
   
   ```bash
   bash-5.0# singularity instance list
   INSTANCE NAMEPID  IP  IMAGE
   busybox  78   /go/busybox_latest.sif
   ```
   And we can exec to it:
   
   ```bash
   bash-5.0# singularity exec instance://busybox echo "HELLO"
   HELLO
   ```
   So, although I'm hesitant that something about travis won't provide us with 
all the permissions that we need, I'm going to give it a try to add a docker 
compose file to reproduce this. My question for you is how I should go about 
providing this integration - the examples given previously expose services, and 
for this case we would want an environment with airflow and singularity in the 
same space. Do you have thoughts or examples for how to approach this?
   
   And then given that we can add a custom container, it would need to use one 
of the Singularity bases (note they are alpine), 

[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-22 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-577337258
 
 
   [Here are my 
thoughts](https://gist.github.com/vsoch/599bd51246c2b2d3da350f97c0916efe) 
@potiuk, I hope that they are helpful! I think you have a challenging problem 
of wanting to scale an interaction that has many expectations around people, 
and human interactions that require kindness and patience. But there definitely 
could be some tweaks (e.g., sticking more to traditional expectations of a 
review, giving details when it's called for and not expecting pre-request 
expertise, and either minimizing tooling that is unexpected, defaulting to 
using the testing environment as the base for giving feedback to the 
contributor, or removing layers of abstraction so it's less confusing).
   
   Anyway, I outlined the details much better in that document. Please feel 
free to share!


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-21 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-576851524
 
 
   Writing up this robust feedback and researching the CI solutions is going to 
take me some time, just to keep you updated I won't be able to work on this 
promptly, I need to prioritize work for my job. But I'll be back, stay tuned! 
:) 


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-20 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-576472830
 
 
   But one quick note - regardless of the testing troubles, the support on this 
PR has been absolutely excellent, night and day when compared to my previous 
experience. I'm really appreciative of that :)


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-20 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-576471260
 
 
   You definitely can’t get a fully working Singularity from within Docker 
(even with privileged it’s a bit janky) so I’ll need to look at the other 
options and decide what is most logical to try first. I’ll take a look 
tomorrow, thanks for the clearly outlined options! 


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-20 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-576470774
 
 
   Ah understood, I thought it was for the PR title, I’ll update the message 
for the next fix I do.
   
   I think the documentation is good, there are just too many moving parts for 
a new contributor. I have limited bandwidth to read into the minute details so 
I tend to be responsive over proactive in terms of reading everything first, 
and I would say this tends to be the case for a large subset of contributors - 
we understand the basics and rely on the CI to tell us what to fix, and don’t 
expect all the other bells and whistles with new tools and steps we have to 
learn for just this PR. For this particular PR I found when I tried to follow 
the docs it was more trouble than just looking at the errors in the CI and 
fixing them one by one. When I largely ignored the docs and just looked at the 
CI and tried to fix things on my own based on that I was finally able to make 
progress. Your links and verbose re-explanation of current docs I know are done 
with good intention, but a large chunk sort of just confused me more.
   
   If it makes you feel better, just chock it up to me being an idiot and I 
promise I’ll never bother this community again.


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-20 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-576459284
 
 
   Also, could you give feedback on:
   
   > Title Validator — Wrong commit title: adding singularity operator and 
tests 
   
   What is wrong about it?


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


With regards,
Apache Git Services


[GitHub] [airflow] vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add singularity to airflow

2020-01-20 Thread GitBox
vsoch commented on issue #7191: [AIRFLOW-4030] second attempt to add 
singularity to airflow
URL: https://github.com/apache/airflow/pull/7191#issuecomment-576459203
 
 
   The linting appears to be all set, but since Singularity is installed, the 
tests are obviously going to fail. Let me know how you would like to proceed - 
it's a non trivial thing to get Singularity installed from within Travis.


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


With regards,
Apache Git Services