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

upstream pull #8

Merged
merged 79 commits into from
Mar 2, 2022
Merged

upstream pull #8

merged 79 commits into from
Mar 2, 2022

Conversation

amadigan
Copy link

@amadigan amadigan commented Mar 2, 2022

rafael-lima-tw and others added 30 commits July 23, 2021 14:22
Bumps [jinja2](https://github.com/pallets/jinja) from 2.11.3 to 3.0.1.
- [Release notes](https://github.com/pallets/jinja/releases)
- [Changelog](https://github.com/pallets/jinja/blob/main/CHANGES.rst)
- [Commits](pallets/jinja@2.11.3...3.0.1)

---
updated-dependencies:
- dependency-name: jinja2
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Peter Kosztolanyi <[email protected]>
* downgrade setuptools to support use_2to3

* fixed install method
* introducing makefile

* updated docker files

* updated documentations and scripts to use makefile

* updated workflow

* update workflow to install only connectors

* fixed wrong place of character

* removed unnecessary target for all_connectors

* fixed to support ubuntu 18

* fixed list variables to be compatibale with ubuntu 18

* updated ci check

* updated help message
koszti and others added 22 commits January 25, 2022 15:02
* added json properties validator

* fixed pep8

* fixed mocked object

* refactoring

* refactoring testcase

* fixed lint

* Update tests/units/cli/test_commands.py

Co-authored-by: Samira El Aabidi <[email protected]>

* using unittest mock

* separate tests

* refactored unit test

* refactored tests

* fixed annotations

Co-authored-by: Samira El Aabidi <[email protected]>
Co-authored-by: Jeet Parekh <[email protected]>
Co-authored-by: Samira El Aabidi <[email protected]>
* release v0.41.0

* Update CHANGELOG.md

* parameterise python in makefile

* Update CHANGELOG.md

Co-authored-by: Amir Mofakhar <[email protected]>

* Update Makefile

Co-authored-by: Amir Mofakhar <[email protected]>
* improve OS signal handling

* change tests

* remove commented code for terminating main process
* Improve signal handling

Only the `bash` process which PipelineWise creates needs to be
terminated. Any other processes are not PipelineWise's responsibility to
kill.

* Move log rename outside `try...except`

* Make sure pidfile is removed

* Guarentee the correct process is killed

* Remove temp files on exit

* Use `pgid` to determine which children to kill

* Add recursive killing back in
* [AP-1109] Fix docker build process in github actions

* Update dockerhub.yml
* [AP-1123] run linting in docker

* [AP-1123] add __name__ check
…lity in favor of internally implemented version
@@ -181,42 +186,37 @@ def load_yaml(yaml_file, vault_secret=None):
secret_file.load()
vault.secrets = [('default', secret_file)]

# YAML ENV VAR
Copy link
Author

Choose a reason for hiding this comment

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

There was a conflict in this file. It looks like the way the YAML configuration is loaded was changed. The changes we made were to enable reading environment variables when loading the config, but it seems pipelinewise has added equivalent functionality internally (https://transferwise.github.io/pipelinewise/installation_guide/creating_pipelines.html#environment-variables-in-yaml-config). I'm submitting a PR to change the syntax in datawarehouse.

Copy link
Author

Choose a reason for hiding this comment

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

Just to be clear, this file is now identical to upstream, I removed any changes we had made.

Comment on lines -201 to +209
file_data = stream.read()
data = yaml.load(file_data, Loader=yaml.Loader)

'''
Commenting code below for posterity. We are not using ansible functionality but yaml load should
follow the same code path regardless of the file encryption state.
'''
#loader = AnsibleLoader(stream, None, vault.secrets)
#try:
# data = loader.get_single_data()
#except Exception as exc:
# raise Exception(f'Error when loading YAML config at {yaml_file} {exc}') from exc
#finally:
# loader.dispose()
loader = AnsibleLoader(stream, None, vault.secrets)
try:
data = loader.get_single_data()
except Exception as exc:
raise Exception(
f'Error when loading YAML config at {yaml_file} {exc}'
) from exc
finally:
loader.dispose()
Copy link
Author

Choose a reason for hiding this comment

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

This is where the conflict was.

@amadigan amadigan requested review from jonathansu and a team March 2, 2022 00:16
@amadigan amadigan self-assigned this Mar 2, 2022
@amadigan amadigan merged commit c82480e into master Mar 2, 2022
@amadigan amadigan deleted the upstream-pull branch March 2, 2022 00:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.