Re: Review Request 73112: ATLAS-4090 - Circular Imports Fix

2020-12-29 Thread Mariusz Górski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73112/
---

(Updated Dec. 29, 2020, 9:36 p.m.)


Review request for atlas and Verdan Mahmood.


Changes
---

change after running sample README.md script


Repository: atlas


Description
---

Fix circular imports due to codependency between glossary and instance models


Diffs (updated)
-

  intg/src/main/python/README.md a97371762 
  intg/src/main/python/apache_atlas/model/instance.py 707d5bbe4 


Diff: https://reviews.apache.org/r/73112/diff/2/

Changes: https://reviews.apache.org/r/73112/diff/1-2/


Testing
---


Thanks,

Mariusz Górski



Re: Review Request 73109: ATLAS-4089 - Refactor Python Atlas Client to PEP8

2020-12-29 Thread Mariusz Górski


> On Dec. 29, 2020, 2:06 a.m., Madhan Neethiraj wrote:
> > Ship It!
> 
> Verdan Mahmood wrote:
> How do you guys verify these changes? How did you test it Madhan? This 
> gives me a ciruclar import errors for each call. 
> Perhaps I missed anything.

I've tested it locally but must've added some changes afterwards. Client has no 
test coverage yet, this should be next step I think to add unit tests run in 
CI. In the meantime please test https://reviews.apache.org/r/73112/ to check if 
that resolved your issue (I've reverted import approach in instance module to 
previous one). It did in my case - tested by running examples in README.md.

The version before refactoring contained several issues fixed in this MR (like 
several functions with same names but different arguments which is a no-go in 
Python).


- Mariusz


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73109/#review222390
---


On Dec. 28, 2020, 12:36 p.m., Mariusz Górski wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73109/
> ---
> 
> (Updated Dec. 28, 2020, 12:36 p.m.)
> 
> 
> Review request for atlas, Madhan Neethiraj and Verdan Mahmood.
> 
> 
> Bugs: ATLAS-4089
> https://issues.apache.org/jira/browse/ATLAS-4089
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> * As agreed in https://reviews.apache.org/r/73097/
> * Autoformatted code to PEP8
> * Fixed linting issues after running flake8
> * Fixed static type issues after running mypy
> 
> 
> Diffs
> -
> 
>   intg/src/main/python/Makefile PRE-CREATION 
>   intg/src/main/python/apache_atlas/__init__.py 4aa54b866 
>   intg/src/main/python/apache_atlas/client/__init__.py f1155945f 
>   intg/src/main/python/apache_atlas/client/admin.py 171bcf8c5 
>   intg/src/main/python/apache_atlas/client/base_client.py fe0ddf40a 
>   intg/src/main/python/apache_atlas/client/discovery.py 5bead22e7 
>   intg/src/main/python/apache_atlas/client/entity.py ebfe27f9a 
>   intg/src/main/python/apache_atlas/client/glossary.py ed7acdf41 
>   intg/src/main/python/apache_atlas/client/lineage.py 980efcd2c 
>   intg/src/main/python/apache_atlas/client/relationship.py 72b701ab1 
>   intg/src/main/python/apache_atlas/client/typedef.py 2ee8f5a9e 
>   intg/src/main/python/apache_atlas/exceptions.py 839befa6c 
>   intg/src/main/python/apache_atlas/model/__init__.py f1155945f 
>   intg/src/main/python/apache_atlas/model/admin.py 3be8a94f5 
>   intg/src/main/python/apache_atlas/model/discovery.py 61a992ce7 
>   intg/src/main/python/apache_atlas/model/enums.py a5064f576 
>   intg/src/main/python/apache_atlas/model/glossary.py c81e333c3 
>   intg/src/main/python/apache_atlas/model/instance.py 89dbe3505 
>   intg/src/main/python/apache_atlas/model/lineage.py 01ba08a11 
>   intg/src/main/python/apache_atlas/model/misc.py 23d0d2fb5 
>   intg/src/main/python/apache_atlas/model/profile.py 4c2770218 
>   intg/src/main/python/apache_atlas/model/relationship.py 58bfd1bd1 
>   intg/src/main/python/apache_atlas/model/typedef.py f6fdf1afd 
>   intg/src/main/python/apache_atlas/utils.py 8384dc555 
>   intg/src/main/python/requirements-dev.txt PRE-CREATION 
>   intg/src/main/python/setup.py f11fa55ad 
>   intg/src/main/python/tox.ini PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/73109/diff/2/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Mariusz Górski
> 
>



Review Request 73112: ATLAS-4090 - Circular Imports Fix

2020-12-29 Thread Mariusz Górski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73112/
---

Review request for atlas and Verdan Mahmood.


Repository: atlas


Description
---

Fix circular imports due to codependency between glossary and instance models


Diffs
-

  intg/src/main/python/apache_atlas/model/instance.py 707d5bbe4 


Diff: https://reviews.apache.org/r/73112/diff/1/


Testing
---


Thanks,

Mariusz Górski



Review Request 73109: ATLAS-4089 - Refactor Python Atlas Client to PEP8

2020-12-28 Thread Mariusz Górski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73109/
---

Review request for atlas, Madhan Neethiraj and Verdan Mahmood.


Bugs: ATLAS-4089
https://issues.apache.org/jira/browse/ATLAS-4089


Repository: atlas


Description
---

* As agreed in https://reviews.apache.org/r/73097/
* Autoformatted code to PEP8
* Fixed linting issues after running flake8
* Fixed static type issues after running mypy


Diffs
-

  intg/src/main/python/Makefile PRE-CREATION 
  intg/src/main/python/apache_atlas/__init__.py 4aa54b866 
  intg/src/main/python/apache_atlas/client/__init__.py f1155945f 
  intg/src/main/python/apache_atlas/client/admin.py 171bcf8c5 
  intg/src/main/python/apache_atlas/client/base_client.py fe0ddf40a 
  intg/src/main/python/apache_atlas/client/discovery.py 5bead22e7 
  intg/src/main/python/apache_atlas/client/entity.py ebfe27f9a 
  intg/src/main/python/apache_atlas/client/glossary.py ed7acdf41 
  intg/src/main/python/apache_atlas/client/lineage.py 980efcd2c 
  intg/src/main/python/apache_atlas/client/relationship.py 72b701ab1 
  intg/src/main/python/apache_atlas/client/typedef.py 2ee8f5a9e 
  intg/src/main/python/apache_atlas/exceptions.py 839befa6c 
  intg/src/main/python/apache_atlas/model/__init__.py f1155945f 
  intg/src/main/python/apache_atlas/model/admin.py 3be8a94f5 
  intg/src/main/python/apache_atlas/model/discovery.py 61a992ce7 
  intg/src/main/python/apache_atlas/model/enums.py a5064f576 
  intg/src/main/python/apache_atlas/model/glossary.py c81e333c3 
  intg/src/main/python/apache_atlas/model/instance.py 89dbe3505 
  intg/src/main/python/apache_atlas/model/lineage.py 01ba08a11 
  intg/src/main/python/apache_atlas/model/misc.py 23d0d2fb5 
  intg/src/main/python/apache_atlas/model/profile.py 4c2770218 
  intg/src/main/python/apache_atlas/model/relationship.py 58bfd1bd1 
  intg/src/main/python/apache_atlas/model/typedef.py f6fdf1afd 
  intg/src/main/python/apache_atlas/utils.py 8384dc555 
  intg/src/main/python/requirements-dev.txt PRE-CREATION 
  intg/src/main/python/setup.py f11fa55ad 


Diff: https://reviews.apache.org/r/73109/diff/1/


Testing
---


Thanks,

Mariusz Górski



Re: Review Request 73097: ATLAS-4086: Python Client Fixes - Basic Search and Bulk Entities using GUIDs

2020-12-22 Thread Mariusz Górski


> On Dec. 17, 2020, 8:27 p.m., Madhan Neethiraj wrote:
> > intg/src/main/python/apache_atlas/model/instance.py
> > Line 70 (original), 74 (patched)
> > 
> >
> > Many coding standards suggest line length of 80, but  often adhering to 
> > this will result in poor readability - like in this case.
> > 
> > I believe these guidelines were applicable for long gone era when folks 
> > printed code on paper, and/or the monitors (green-text?) were too narrow to 
> > render long lines.
> > 
> > Wide monitors, powerful IDEs and lack of the need to print code on 
> > paper should enable removing this unnecessary line length limit, and 
> > significantly improve code readability.
> > 
> > I strongly suggest to revert all such changes.
> 
> Verdan Mahmood wrote:
> I agree on this. as the only reason to specify the line length is to 
> enforce the consistency. And if we agree on one line length, I am perfectly 
> fine with this. BUT in any case, we should specify the maximum line length. 
> For many new projects, we are using `120` as the max line length.
> 
> Madhan Neethiraj wrote:
> 120 is much better than 80. However, the key is readablity than complying 
> to a number.

I think another angle in this discussion would be to look at Python client from 
community and developers perspective. 

PEP is a de-facto standard for Python formatting and if we want encourage 
Pythonistas to develop/extend it then we should consider how they work. While 
developing Python code in JVM-native IDE (like Eclipse or IntelliJ) won't raise 
any flags, these minor things will be a pain points for any IDE designed to 
work with Python (like Pycharm) - any kind of deviation from standard looks 
messy while developing. And if Pythonista is reviewing the code, then he/she 
will have much easier job looking at a familiar syntax. Why not make this part 
of Atlas familiar to them ? If full set of best practices/tools around Python 
development (like linting, static type validation) was to be enabled (and imo 
it should) there is no way around it really.

I don't mean to start off yet another argument like 'spaces vs tabs' but having 
experience with different Open Source projects enabling clients for multiple 
languages I always appreciated when every client is designed and aligned with 
standards applicable to it and familiar to native users. In the end, every 
contribution should be for a good of end users, which might come from differnt 
backgrounds.

Having said that, I think fixing code anf reformatting it should be two 
separate PRs so one doesn't block another.


- Mariusz


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/73097/#review222360
---


On Dec. 17, 2020, 1:45 p.m., Verdan Mahmood wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/73097/
> ---
> 
> (Updated Dec. 17, 2020, 1:45 p.m.)
> 
> 
> Review request for atlas, Madhan Neethiraj, Pinal Shah, and Sarath 
> Subramanian.
> 
> 
> Bugs: ATLAS-4086
> https://issues.apache.org/jira/browse/ATLAS-4086
> 
> 
> Repository: atlas
> 
> 
> Description
> ---
> 
> Python Client Fixes
> 
> 
> Diffs
> -
> 
>   intg/src/main/python/apache_atlas/client/discovery.py 
> 5bead22e776c00988e7b19b0e6e81e8a3d3175df 
>   intg/src/main/python/apache_atlas/model/discovery.py 
> 61a992ce7a86f3f4c75792cfe66d87dac8ca991c 
>   intg/src/main/python/apache_atlas/model/instance.py 
> 89dbe350587d66442e599598908179c6ac8334d5 
> 
> 
> Diff: https://reviews.apache.org/r/73097/diff/1/
> 
> 
> Testing
> ---
> 
> - This fixes the indentation according to pep8. 
> - Fixes the basic search using post method, it was broken. 
> - Getting Bulk entities using GUIDs was broken, it also fix that.
> 
> 
> Thanks,
> 
> Verdan Mahmood
> 
>



Re: Review Request 72104: ATLAS-3610 Enable logging to STDOUT in Atlas startup scripts

2020-03-26 Thread Mariusz Górski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72104/
---

(Updated March 26, 2020, 7:41 a.m.)


Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Madhan Neethiraj, 
Nixon Rodrigues, and Sarath Subramanian.


Changes
---

Updating Testing Done section


Repository: atlas


Description
---

This patch enables logging to STDOUT from processes spawned through 
atlas_start.py or atlas_config.py scripts. For now, even if configured in 
log4j, logging to STDOUT was supressed (by redirecting to log files only).


Diffs
-

  distro/pom.xml 7159b16cf 
  distro/src/bin/atlas_config.py f09026ff9 
  distro/src/bin/atlas_start.py 963faf402 
  distro/src/bin/cputil.py 98a9dc36d 
  distro/src/conf/atlas-env.sh c4241e665 
  distro/src/test/python/scripts/TestMetadata.py f1235f747 


Diff: https://reviews.apache.org/r/72104/diff/4/


Testing (updated)
---

The tests were concluded by:

1. Building & running Atlas locally with default log4j configuration and 
-Dlog.console mvn build option set to true.
2. Building & running Atlas on Openshift with log4j configuration pushing 
everything to STDOUT and -Dlog.console mvn build option set to true.
3. Running distro tests on localhost

In both scenarios logs were available both in the .out/.err files in the Atlas 
logdir (backwards compatibility), but at the same time I was able to see them 
in STDOUT of both running process (local installation) and pod (openshift 
deployment).


Thanks,

Mariusz Górski



Re: Review Request 72104: ATLAS-3610 Enable logging to STDOUT in Atlas startup scripts

2020-02-27 Thread Mariusz Górski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72104/
---

(Updated Feb. 27, 2020, 2:39 p.m.)


Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Madhan Neethiraj, 
Nixon Rodrigues, and Sarath Subramanian.


Changes
---

fix failing tests


Repository: atlas


Description
---

This patch enables logging to STDOUT from processes spawned through 
atlas_start.py or atlas_config.py scripts. For now, even if configured in 
log4j, logging to STDOUT was supressed (by redirecting to log files only).


Diffs (updated)
-

  distro/pom.xml 7159b16cf 
  distro/src/bin/atlas_config.py f09026ff9 
  distro/src/bin/atlas_start.py 963faf402 
  distro/src/bin/cputil.py 98a9dc36d 
  distro/src/conf/atlas-env.sh c4241e665 
  distro/src/test/python/scripts/TestMetadata.py f1235f747 


Diff: https://reviews.apache.org/r/72104/diff/4/

Changes: https://reviews.apache.org/r/72104/diff/3-4/


Testing
---

The tests were concluded by:

1. Building & running Atlas locally with default log4j configuration and 
-Dlog.console mvn build option set to true.
2. Building & running Atlas on Openshift with log4j configuration pushing 
everything to STDOUT and -Dlog.console mvn build option set to true.

In both scenarios logs were available both in the .out/.err files in the Atlas 
logdir (backwards compatibility), but at the same time I was able to see them 
in STDOUT of both running process (local installation) and pod (openshift 
deployment).


Thanks,

Mariusz Górski



Re: Review Request 72104: ATLAS-3610 Enable logging to STDOUT in Atlas startup scripts

2020-02-22 Thread Mariusz Górski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72104/
---

(Updated Feb. 22, 2020, 3:30 p.m.)


Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Madhan Neethiraj, 
Nixon Rodrigues, and Sarath Subramanian.


Changes
---

updated testing description, added reviewers


Repository: atlas


Description
---

This patch enables logging to STDOUT from processes spawned through 
atlas_start.py or atlas_config.py scripts. For now, even if configured in 
log4j, logging to STDOUT was supressed (by redirecting to log files only).


Diffs
-

  distro/pom.xml 7159b16cf 
  distro/src/bin/atlas_config.py f09026ff9 
  distro/src/bin/atlas_start.py 963faf402 
  distro/src/bin/cputil.py 98a9dc36d 
  distro/src/conf/atlas-env.sh c4241e665 


Diff: https://reviews.apache.org/r/72104/diff/3/


Testing (updated)
---

The tests were concluded by:

1. Building & running Atlas locally with default log4j configuration and 
-Dlog.console mvn build option set to true.
2. Building & running Atlas on Openshift with log4j configuration pushing 
everything to STDOUT and -Dlog.console mvn build option set to true.

In both scenarios logs were available both in the .out/.err files in the Atlas 
logdir (backwards compatibility), but at the same time I was able to see them 
in STDOUT of both running process (local installation) and pod (openshift 
deployment).


Thanks,

Mariusz Górski



Re: Review Request 72104: Enable logging to STDOUT in Atlas startup scripts

2020-02-15 Thread Mariusz Górski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72104/
---

(Updated Feb. 15, 2020, 12:51 p.m.)


Review request for atlas and Bolke de Bruin.


Repository: atlas


Description
---

This patch enables logging to STDOUT from processes spawned through 
atlas_start.py or atlas_config.py scripts. For now, even if configured in 
log4j, logging to STDOUT was supressed (by redirecting to log files only).


Diffs (updated)
-

  distro/pom.xml 7159b16cf 
  distro/src/bin/atlas_config.py f09026ff9 
  distro/src/bin/atlas_start.py 963faf402 
  distro/src/bin/cputil.py 98a9dc36d 
  distro/src/conf/atlas-env.sh c4241e665 


Diff: https://reviews.apache.org/r/72104/diff/3/

Changes: https://reviews.apache.org/r/72104/diff/2-3/


Testing
---

The tests were concluded by:

1. Building & running Atlas locally with default log4j configuration and 
ENABLE_LOGGING_TO_CONSOLE variable set to true.
2. Building & running Atlas on Openshift with log4j configuration pushing 
everything to STDOUT and ENABLE_LOGGING_TO_CONSOLE variable set to true.

In both scenarios logs were available both in the .out/.err files in the Atlas 
logdir (backwards compatibility), but at the same time I was able to see them 
in STDOUT of both running process (local installation) and pod (openshift 
deployment).


Thanks,

Mariusz Górski



Re: Review Request 72104: Enable logging to STDOUT in Atlas startup scripts

2020-02-13 Thread Mariusz Górski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72104/
---

(Updated Feb. 13, 2020, 6:19 p.m.)


Review request for atlas and Bolke de Bruin.


Repository: atlas


Description
---

This patch enables logging to STDOUT from processes spawned through 
atlas_start.py or atlas_config.py scripts. For now, even if configured in 
log4j, logging to STDOUT was supressed (by redirecting to log files only).


Diffs (updated)
-

  distro/src/bin/atlas_admin.py 894e4da03 
  distro/src/bin/atlas_config.py f09026ff9 
  distro/src/bin/atlas_kafka_setup.py 146a7e509 
  distro/src/bin/atlas_kafka_setup_hook.py 6fdff740d 
  distro/src/bin/atlas_start.py 963faf402 
  distro/src/bin/atlas_update_simple_auth_json.py f93207124 
  distro/src/bin/cputil.py 98a9dc36d 
  distro/src/bin/quick_start.py 9e8b33c54 
  distro/src/bin/quick_start_v1.py e9997b1a2 


Diff: https://reviews.apache.org/r/72104/diff/2/

Changes: https://reviews.apache.org/r/72104/diff/1-2/


Testing
---

The tests were concluded by:

1. Building & running Atlas locally with default log4j configuration and 
ENABLE_LOGGING_TO_CONSOLE variable set to true.
2. Building & running Atlas on Openshift with log4j configuration pushing 
everything to STDOUT and ENABLE_LOGGING_TO_CONSOLE variable set to true.

In both scenarios logs were available both in the .out/.err files in the Atlas 
logdir (backwards compatibility), but at the same time I was able to see them 
in STDOUT of both running process (local installation) and pod (openshift 
deployment).


Thanks,

Mariusz Górski



Review Request 72104: Enable logging to STDOUT in Atlas startup scripts

2020-02-10 Thread Mariusz Górski

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72104/
---

Review request for atlas and Bolke de Bruin.


Repository: atlas


Description
---

This patch enables logging to STDOUT from processes spawned through 
atlas_start.py or atlas_config.py scripts. For now, even if configured in 
log4j, logging to STDOUT was supressed (by redirecting to log files only).


Diffs
-

  distro/src/bin/atlas_config.py f09026ff9 
  distro/src/bin/atlas_start.py 963faf402 


Diff: https://reviews.apache.org/r/72104/diff/1/


Testing
---

The tests were concluded by:

1. Building & running Atlas locally with default log4j configuration and 
ENABLE_LOGGING_TO_CONSOLE variable set to true.
2. Building & running Atlas on Openshift with log4j configuration pushing 
everything to STDOUT and ENABLE_LOGGING_TO_CONSOLE variable set to true.

In both scenarios logs were available both in the .out/.err files in the Atlas 
logdir (backwards compatibility), but at the same time I was able to see them 
in STDOUT of both running process (local installation) and pod (openshift 
deployment).


Thanks,

Mariusz Górski