Skip to content

Rework scripts - #10

Open
felixjeskeleanix wants to merge 39 commits into
masterfrom
rework-scripts
Open

felixjeskeleanix wants to merge 39 commits into
masterfrom
rework-scripts

Conversation

@felixjeskeleanix

Copy link
Copy Markdown

Rework of the public scripts.
Changes:

  • Remove lxpy client
  • Remove unfit scripts which use mtm
  • Add environmental variables for the input values
  • Add logging
  • Add docstrings

Not yet updated: startIntegrationAPIRun

Reviewer: kostas.petrakis@leanix.net

@kostas-petrakis kostas-petrakis left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work @felixjeskeleanix!
I start with a draft of comments, so we don't overwhelm the entire PR, then we can move on.
Can you also align the corrections for the rest of the scripts?
P.S Please delete the .DS_Store file

Comment thread archiveFactSheets/archive.py Outdated
timeout=TIMEOUT)
response.raise_for_status()
access_token = response.json()['access_token']
auth_header = 'Bearer ' + access_token

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It is better to use f string interpolation :)

Suggested change
auth_header = 'Bearer ' + access_token
auth_header = f'Bearer {access_token}'

Comment thread archiveFactSheets/archive.py Outdated
Example:
$ python archiveFactsheets.py

Attributes:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In Python we usually use the term Global variables to describe variables used in the context of constants.

Suggested change
Attributes:
Global variables:

Comment thread archiveFactSheets/archive.py Outdated
response = call(query)
print (response)
""" % (id)
logging.info("delete " + id)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Let's help the consumers find their way in the logs. Also note that it is preferred to use single quotes, or at least not mix double with single quotes.

Suggested change
logging.info("delete " + id)
logging.info(f'Archiving fact sheet with id: {id}')

Comment thread archiveFactSheets/archive.py Outdated
""" % (id)
logging.info("delete " + id)
response = call(query, header, LEANIX_REQUEST_URL)
logging.info(response)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It is better to use debug for responses as those usually are not meant to be logged (expensive and noisy).

Suggested change
logging.info(response)
logging.debug(response)

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.

2 participants