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 upstream up to netflix/metaflow 2.5.1 #155

Closed
wants to merge 185 commits into from

Conversation

cloudw
Copy link
Collaborator

@cloudw cloudw commented Feb 17, 2022

Next steps:

  • Resolve merge conflicts in a separate PR.
  • Review important changes that takes longer. Creates backlog tickets.

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 <[email protected]>
…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 24 commits January 17, 2022 17:20
…x#822)

* Store information about the DAG being executed in an artifact

This will allow the UI and the default card to easily get the DAG
even if the code is not packaged.

TODO: Provide methods in the client

* Addressed comments and cleaned up vestigial code

* Fix typo

Also moved up a variable initialization to make it more robust

* Addressed comments; added support for parallel
* Fix issue when running on batch with local metadata

Artifacts were persisted *after* the metadata was saved to S3 to be
repatriated locally which caused it to completely miss.

This addresses this issue (fixes Netflix#878) and also addresses another issue
in load_metadata when loading non existent metadata.

* Addressed comments
* [WIP] Very initial support for namespace packages for metaflow_extensions.

This is more to get an early flavor. Does not yet work.

* First working version

* Updates to extensions framework:
  - Now with more debugging; set METAFLOW_DEBUG_EXT=1
  - More validation to avoid unexpected issues (conflicts between
    packages for example). Also verify if an "old" metaflow_extensions
    is around. The user is warned with a comprehensible (hopefully) error
    message.
  - Support for packages in PYTHONPATH
  - Updated test case

* Addressed comments

* Fix issue when there are no metaflow_extensions packages

* Forgot to commit a file that caused test failures

Also a bit more code cleanup to eliminate repetition
* Vendor importlib_metadata

* format vendor.py with black

* add attribution

* adjust precommit hook

* format
* Fix compute_resources to take into account string values

* Always return strings.

* additional unit tests

Co-authored-by: Oleg Avdeev <[email protected]>
* 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 <[email protected]>
Co-authored-by: Brendan Gibson <[email protected]>
Co-authored-by: adam <[email protected]>
Co-authored-by: Romain Cledat <[email protected]>

* 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 <[email protected]>
Co-authored-by: Brendan Gibson <[email protected]>
Co-authored-by: adam <[email protected]>
Co-authored-by: Romain Cledat <[email protected]>
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] <[email protected]>

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] <[email protected]>

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 <[email protected]>
- 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.
@cloudw cloudw self-assigned this Feb 17, 2022
@cloudw
Copy link
Collaborator Author

cloudw commented Feb 17, 2022

Note for myself on test coverage change: Netflix#929.

@cloudw
Copy link
Collaborator Author

cloudw commented Mar 28, 2022

Closing in favor of merge_upstream_2_5_3 branch

@cloudw cloudw closed this Mar 28, 2022
@cloudw cloudw deleted the merge_upstream_2_5_1 branch April 5, 2022 18:14
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.