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

Merge Netflix/metaflow to version 2.5.3 with fixes #162

Closed
wants to merge 200 commits into from

Conversation

cloudw
Copy link
Collaborator

@cloudw cloudw commented Mar 28, 2022

Resolving conflicts from upstream.

Most of conflicts are result of netflix/metaflow embracing black formatting 🎉

Major changes that resulted in code changes:

romain-intel and others added 30 commits July 14, 2021 16:50
)

When you override an existing module inside a metaflow custom
package, we now create a module suffixed with '_orig' (so, for
example, if you override 'metaflow.plugins.aws', you can access
the original module as 'metaflow.plugins.aws._orig'
…ge (Netflix#614)

* Allow Metaflow Custom packages to define additional suffixes to package

This enables you to package non-python files in your metaflow_custom package
and have them included when Metaflow creates the code package.

* Addressed comment
* Add inputs to task_pre_step

* Addressed comments

* Forgot to remove an import
* Refactor @resources decorator

@resources decorator is shared by all compute related decorators -
@Batch, @lambda, @K8s, @titus. This patch moves it out of
batch_decorator.py so that other decorators can cleanly reference
it.

* Update __init__.py
You can now specify a function that returns an AWS client. This is
useful if you want to use something other than boto3 and can be used
with the metaflow_custom mechanism to provide your own authentication
* Add S3 tests

* Addressed comments
…flix#630)

A parameter specification like -
```
Parameter(name="test_param", type=int, default=None)
```
will result in an error even though the default has been specified
```
Flow failed:
    The value of parameter test_param is ambiguous. It does not have a default and it is not required.
```

This PR fixes this issue.
* DRAFT: IncludeFile now returns the included file in the client and CLI

THIS IS NOT FINISHED; DO NOT MERGE AS IS.

* Fix the tests

* Forgot to update type check for multiple encoding
* Add resource tags to AWS Batch jobs

This PR assumes that Batch:TagResource is whitelisted for the
role which submits the AWS Batch job.

* propagate tags

* add env var

* better commens

* add production token
…flix#623)

* Properly allow full toplevel module overrides in metaflow_custom

* Modified test to follow new format

* Improve module aliasing for metaflow_custom (Netflix#625)
* Handle parameters as `foreach vars`

* Change fix to work more upstream

The reason _find_input was not seeing the proper value for the parameters
was because _init_parameters, which is supposed to initialize them, calls
_get_parameters which needlessly calls input which calls _find_input. This
removes the needless call to input which solves the problem and has the
added benefit of being slightly more efficient.

Co-authored-by: Romain Cledat <rcledat@netflix.com>
…flix#660)

* Conda environment now applys underlying environment's decorators

* Add tuples instead of unpacking with * for py2 compatibility
* Added test for exception handling.
- Added Repr to fix error representation problem

* Fix test doc string

* nit fixes.
…x#674)

* Fixes recursion error when METAFLOW_DEFAULT_ENVIRONMENT=conda

* Add explanation
romain-intel and others added 28 commits January 25, 2022 12:21
* Fix compute_resources to take into account string values

* Always return strings.

* additional unit tests

Co-authored-by: Oleg Avdeev <oleg.v.avdeev@gmail.com>
* multiple decorator support

* Fixing comments

* comment fix.

* commet fix

* allow multiple decorators of same type from cli

* Stacked @card v1.2 : Customizing `@card` with `current.card` (Netflix#894)

* allow passing userland `MetaflowCardComponents`
-  Added api for `current.cards` that handles multiple decorators

* `current.cards` with `id` support.
- Introduced `ALLOW_USER_COMPONENTS` attribute to `MetaflowCard` class
- setting `ALLOW_USER_COMPONENTS=True` allow editable cards
- Modified logic of `CardComponentCollector`
- `CardComponentCollector.append` only accessible to default editble card

* Added `customize=True` in `@card`
- `customize=True` only allowed for one @card deco per @step
- `customize=True` sets the card to be default editable
- `current.card.append` appends to @card with `customize=True`


* Stacked @card v1.2: Read cli changes for Supporting Multiple`@card`s (Netflix#895)

* Added the `--id` option to `card view`/`card get`
- modified datastore and fixed the exception class.
- Allowing some ambiguity in pathspec argument for `get/view` command.

* Added `id` argument to `get_cards`

* Bringing `card list` cli functionality from test-suite branch
- added functionality to list cards about a task and even list it as JSON.

* changed hash checking logic.
- instead of equating we check `startswith`

* Added list many feature for `card list`
- listing all cards from the latest run when card list is called with no argument

* Added `card list` print formatting changes
- `--as-json` works for many cards and single cards


* Stacked @card v1.2 : New `MetaflowCardComponent`s  (Netflix#896)

* user-facing `MetaflowCardComponent`s
- import via `from metaflow.cards import Artifact,..`

* 7 Major Changes:
- Removed `Section` component from `metaflow.cards`
- Making user-facing component inherent to `UserComponent` which in turn inherits `MetaflowCardComponent`
- Wrap all `UserComponents` inside a section after rendering everything per card
- created a `render_safely` decorator to ensure fail-safe render of `UserComponent`s
- removed code from component serializer which used internal components
- Refactored some components that return render
- Added docstrings to all components.

* JS + CSS + Cards UI Build

* Stacked @card v1.2 : Graph Related Changes to card cli (Netflix#911)

* accomodating changes from Netflix#833
- Minor tweaks to `graph.py`

* Stacked @card v1.2 : Namespace Packages with `@card` (Netflix#897)

* setup import of cards from `metaflow_extensions` using `metaflow.extension_support`
- Added import modules in `card_modules`
- Added hook in card decorators to add custom packages

* Added some debug statements for external imports.

* Stacked @card v1.2 : Test cases for Multiple `@card`s (Netflix#898)

* Multiple Cards Test Suite Mods (#27)

- Added `list_cards` to `CliCheck` and `MetadataCheck`
- Bringing Netflix#875 into the code
- Added a card that prints taskspec with random number

* Added test case for multiple cards

* Added tests card, summary :
- `current.cards['myid']` should be accessible when cards have an `id` argument in decorator
- `current.cards.append` should not work when there are no single default editable card.
- if a card has `ALLOW_USER_COMPONENTS=False` then it can still be edited via accessing it with `id` property.
- adding arbitrary information to `current.cards.append` should not break user code.
- Only cards with `ALLOW_USER_COMPONENTS=True` are considered default editable.
- If a single @card decorator is present with `id` then it `current.cards.append` should still work
- Access of `current.cards` with non existant id should not fail.
- `current.cards.append` should be accessible to the card with `customize=True`.
-

* fixed `DefaultEditableCardTest` test case
- Fixed comment
- Fixed the `customize=True` test case.

* ensure `test_pathspec_card` has no duplicates
- ensure entropy of rendered information is high enough to not overwrite a file.

* test case fix : `current.cards` to `current.card`

* Added Test case to support import of cards.
- Test case validates that broken card modules don't break metaflow
- test case validates that we are able to import cards from metaflow_extensions
- Test case validate that cards can be editable if they are importable.

* Added Env var to tests to avoid warnings added to cards.

* Added Test for card resume.

* Stacked @card v1.2: Card Dev Docs (Netflix#899)

Co-authored-by: Brendan Gibson <brendan@outerbounds.co>
Co-authored-by: Brendan Gibson <93726128+obgibson@users.noreply.github.com>
Co-authored-by: adam <203779+seethroughdev@users.noreply.github.com>
Co-authored-by: Romain Cledat <rcledat@netflix.com>

* Nit fix in error.

* Fixing bug in decorator

* whitespace.

* Changing import scheme of warning variable.

* fixing warning suppression env var setting

Co-authored-by: Brendan Gibson <brendan@outerbounds.co>
Co-authored-by: Brendan Gibson <93726128+obgibson@users.noreply.github.com>
Co-authored-by: adam <203779+seethroughdev@users.noreply.github.com>
Co-authored-by: Romain Cledat <rcledat@netflix.com>
Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.14.6 to 1.14.7.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.14.6...v1.14.7)

---
updated-dependencies:
- dependency-name: follow-redirects
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…ix#926)

Bumps [marked](https://github.com/markedjs/marked) from 4.0.8 to 4.0.10.
- [Release notes](https://github.com/markedjs/marked/releases)
- [Changelog](https://github.com/markedjs/marked/blob/master/.releaserc.json)
- [Commits](markedjs/marked@v4.0.8...v4.0.10)

---
updated-dependencies:
- dependency-name: marked
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Removing some code bits.

* removed `RENDER_TEMPLATE_PATH`

* Refactoring redundant code.
* Add mamba option

* black format

* fixups

* apply black

* remove disable check

* add support for micromamba

* more changes

Co-authored-by: jimgoo <jimmiegoode@gmail.com>
- Added a card to get the default card JSON object.
- Added Base64 transform to js
- Built JS + CSS/
* vend pylint

* v

* dependency fixes

* update setup.py

* formatting fixes

* update tests

* remove coverage

* fix spurious commit

* address comments

* address black issues

* address comments
* porting changes.

* porting changes.

* Fix

* fix

* Py3.5 accoomodating fix.

* version fix.

* test

* Removing python version 2

* Nit fixes + add/remove py/R versions

* Removing 3.11 py

* testing GH action-reusabitity

* Importing Testing. Deleting file post tests.

* Testing fix

* removing testing dummy file.
- files pointed by symlinks are now dereferenced by tar (no symlinks inside tar)
 - symlink cycles are detected
* Add README.md to PyPI package description

* make black happy
* Don't explicitly break py2 support

* Typo

* Typo
…the Env Escape server (Netflix#960)

We were currently not reflecting possible values passed using PYTHONPATH or
programmatically added to the interpreter launching the environment escape server.
Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.14.7 to 1.14.9.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.14.7...v1.14.9)

---
updated-dependencies:
- dependency-name: follow-redirects
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…Netflix#972)

* Fix "Too many symbolic links" error when using Conda + Batch on MacOS

* Ran black
* Fixed dual distribution import in some cases; fixed get_plugin_cli extension

If sys.path contains the same directory multiple times, metadata.distributions()
will list a package multiple times which breaks the import mechanism we have. This
is addressed by skipping duplicate packages.

get_plugin_cli for extensions wasn't evaluated late enough therefore
not allowing additional parameters to be added.

* Fix issue with metaflow_extensions and Conda environment

In a Conda environment, it is not possible to re-resolve all the
metaflow_extension packages since they appear as a single directory.
In that case, we resolve on the INFO file to give us the proper
information.

Also fixed the way metaflow_extensions are added to the Conda
environment to avoid leaking more information than needed.

* Validate configuration files for extensions and de-duplicate for internal ns packages

* Update packaging of metaflow_extensions packages

Packaging is now handled directly by extension_support.py and
each distribution/package can define its own suffixes to include.

* Rework the import mechanism for modules contained in metaflow_extensions

This change does away with the deprecated load_module and also improves
handling of loading children modules and _orig modules

* Address comments

* Squash merge of origin/master

* Fix issue when packages were left out

* Merge get_pinned_conda_libs

* Fix issue with __init__.py in non-distribution packages

* Moving card import changes on top of Romain's branch (Netflix#973)

* Moving card import changes on top of Romain's branch
- tiny bug fix to extensions.
- Added card related refactor to support mfextinit and regular package.
- removing card packaging related code in decorator + everywhere else.

* Tiny bug fix on top of romain's new changes.

* removing unneccessary logic

* Added a bit more inline documentation and addressed documentation comments

* Properly handle parsing of package requirements

* Fix Pylint errors in some case of metaflow_extensions module aliasing

* Properly handle case of files at the root of PYTHONPATH package

* Tests for Extensions  (Netflix#978)

* Added tests for extensions. Checking if they work.

* dummy commit to see if things work.

* fix

* debug

* bug fix.

* possible fix.

* tweeking context

* Bug fix to tests.

* dummy commit

* bug fix

* Added extension test to core tests.
- remove seperate gh action

* removing files.

* fix

* added extension test to py3 context
- remove redundant complexity.

Co-authored-by: Valay Dave <valaygaurang@gmail.com>
* Fixing some hacky plumbing for card tests.
- Added `--file` arg to card list cli
- Using files to get the information written by cli
- changes to test to use files instead of stdout
- Piping stderrors to stderrr to capture card not found errors.

* Removing `\n` from all card tests assertions
- `\n` was there because earlier we read stdout.
- Now since we read files, it will not be needed.
@cloudw cloudw closed this Mar 28, 2022
@cloudw cloudw deleted the merge_2_5_3 branch April 5, 2022 18:13
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.