Update dependency management workflow #3771

csadorf · 2020-02-19T17:55:10Z

Initial pass at starting to update the dependency workflow.

csadorf · 2020-02-20T10:38:13Z

@ltalirz I'm investigating the CI issues right now.

ltalirz · 2020-02-20T11:03:24Z

@csadorf I've started pinning down the postgres issue, see my post here #3762

The wrapt issue in pre-commit is easy to fix - just replace the dependency by wrapt~=1.11.1

csadorf · 2020-02-20T15:14:15Z

Looks like all directly PR-related issues are resolved here. I'll wait for the postgres CI issue to be resolved upstream.

ltalirz

Thanks a lot @csadorf !
CI tests are fixed now - I'm sure we can get this Pr through next week.

Until then one question from my side

.github/CODEOWNERS

ltalirz

thanks @csadorf - a few comments from my side.

Furthermore, I read AEP 002 in such a way that we should actually provide a working requirements.txt file, and I suggest we commit it to the aiida repository (perhaps something to discuss with @sphuber).
This will be important to ensure that we are still able to pip install aiida-core 1.1.0, say, in 2 years.

This should come with a CI test that installs aiida using the fixed requirements.txt.
Note: This also highlights the limitation of the pip freeze approach over others (like pipfile / ...), namely that there is no concept of the extra_requirements, i.e. we have to decide which of them to include (if any).

utils/dependency_management.py

sphuber · 2020-03-03T13:43:25Z

Is it really necessary to vendor toml?

csadorf · 2020-03-03T13:44:07Z

Is it really necessary to vendor toml?

Of course not, but is there a strong reason against it?

sphuber · 2020-03-03T13:47:19Z

Is it really necessary to vendor toml?

Of course not, but is there a strong reason against it?

Well it adds a lot of code to the repository that also has to then be maintained as well. Is this just necessary for the build system? If so, why can't this just be a dependency?

csadorf · 2020-03-03T13:50:48Z

Is it really necessary to vendor toml?

Of course not, but is there a strong reason against it?

Well it adds a lot of code to the repository that also has to then be maintained as well. Is this just necessary for the build system? If so, why can't this just be a dependency?

It's a dependency of the dependency_management.py script. I will need to install it everywhere I want to use that script. It's what other build-related packages do as well (like setuptools, etc.).

I'll just remove it since this appears to be an issue.

codecov-io · 2020-03-04T17:45:39Z

Codecov Report

Merging #3771 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #3771   +/-   ##
========================================
  Coverage    77.16%   77.16%           
========================================
  Files          458      458           
  Lines        33770    33770           
========================================
  Hits         26058    26058           
  Misses        7712     7712

Flag	Coverage Δ
#django	`69.2% <ø> (ø)`	⬆️
#sqlalchemy	`70.02% <ø> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 035fbba...75f609a. Read the comment docs.

To automatically trigger a review request from the DM for changes that modify files that are related to dependency specification.

To better reflect the broader intended scope.

All, but one of the dependencies (sqlalchemy-utils) are available via conda-forge and mixing channels is generally not advisable.

… script. - And remove equivalent function from validate_consistency script. - Update pre-commit-hooks to reflect change.

csadorf · 2020-03-09T16:41:10Z

The failing pre-commit jobs will be resolved with #3837 .

ltalirz

Thanks, there is one small typo that should be corrected.
Apart from that, just some minor left-over comments from my previous review

ltalirz · 2020-03-09T17:27:44Z

.github/workflows/test-install.yml

+      - 'environment.yml'
+      - 'requirements*.txt'
+      - 'pyproject.toml'
+      - '.github/workflows/test-install.sh'


Suggested change

- '.github/workflows/test-install.sh'

- '.github/workflows/test-install.yml'

Since the environments only differ across python versions, not backends.

The recent update to Click (7.1) breaks our tests and this CI workflow, because the help output formatting was slightly changed. Until we have resolved other issues we should pin the Click version to 7.0.

ltalirz

Alright, let's go!

csadorf · 2020-03-09T18:01:39Z

Alright, let's go!

@ltalirz I'll finalize and merge. 👍

This reverts commit 1bb8351.

csadorf · 2020-03-09T18:24:44Z

@ltalirz I am very sorry, I still needed to remove the trigger for running all workflows on this branch. I did not realize that we had automatic dismissal of review approvals enabled.

It appears that unfortunately the latest run is having some (unrelated?) issues. They can probably be addressed by simply restarting the workflows.

Please merge at your discretion; this is the proposed commit-message for the squashed merge:

Revise dependency management workflow in accordance with AEP 002.

Use .github/CODEOWNERS file to trigger reviews from
dependency-manager team.

All jobs related to testing the installation of aiida-core with
different tools (pip, conda) on different python versions and backends
are moved into the dedicated 'install-tests' workflow.

The test jobs run as part of the continuous integration on every push
are executed against pinned environments (e.g.
'requirements/requirement-py-37.txt').

The 'install-tests' workflow is triggered for the 'master', 'develop',
and release branches ('release/*') as well as branches related to
dependency-management (prefixed with 'dm/'). In addition, the workflow
is triggered nightly to ensure that changes within the Python ecosystem
that break our toolchain and possibly even tests, are automatically
detected within 24 hours.

A new 'update-requirements' workflow is run on release branches and
automatically creates a pull request with revised versions of
requirements files.

The issues caused by pymatgen's use of setup_requires and previously
addressed by installing numpy prior to aiida by default, are now
addressed by specifically checking the setuptools version for Python
3.5 only. We fail the installation process with a descriptive message
to the user in case that the installed setuptools version is
insufficient.

All dependency-management related utility functions, such as
generating and validating non-authoritive dependency specification files
(e.g. 'environment.yml') are concentrated into the
'util/dependency_management.py' scripting file.

The complete workflow is described in detail as part of the Dependency
Management page within the wiki.

csadorf · 2020-03-09T18:51:40Z

@italirz looks like it's ready to merge.

ltalirz · 2020-03-09T18:56:14Z

Ehm... what happened there?

Looks like your PR is cursed ;-)

ltalirz · 2020-03-09T18:56:54Z

Ah, I see - was just the wrong default setting

ltalirz · 2020-03-09T19:04:21Z

Interesting - it seems like the new workflows are immediately applied on existing open PRs

csadorf · 2020-03-09T19:24:26Z

I think they're yellow, because I changed the expected tests. PR authors will need to either rebase or merge develop to trigger the execution of the new workflows.

csadorf requested a review from ltalirz February 19, 2020 17:58

ltalirz reviewed Feb 20, 2020

View reviewed changes

.github/CODEOWNERS Outdated Show resolved Hide resolved

ltalirz reviewed Mar 2, 2020

View reviewed changes

utils/dependency_management.py Outdated Show resolved Hide resolved

utils/dependency_management.py Outdated Show resolved Hide resolved

utils/dependency_management.py Outdated Show resolved Hide resolved

utils/dependency_management.py Outdated Show resolved Hide resolved

csadorf changed the title ~~Misc/dependency management~~ [WIP] Update dependency management workflow Mar 3, 2020

csadorf force-pushed the misc/dependency-management branch 2 times, most recently from 8948662 to 060fde4 Compare March 4, 2020 17:31

csadorf force-pushed the misc/dependency-management branch 3 times, most recently from 3d9bfb5 to 564ff22 Compare March 5, 2020 15:11

csadorf added 13 commits March 5, 2020 16:13

Add .github/CODEOWNERS file.

b1c7270

To automatically trigger a review request from the DM for changes that modify files that are related to dependency specification.

Rename script 'update_dependencies' to 'dependency_management'.

070a78d

To better reflect the broader intended scope.

Remove the 'etetoolkit' channel from environment.yml.

c7601ed

All, but one of the dependencies (sqlalchemy-utils) are available via conda-forge and mixing channels is generally not advisable.

Implement validate command for dependency management util script.

5d552ea

Add explicit dependency validation step to CI.

f6604bc

Fix yaml requirement.

13af1d3

Implement 'generate-conda-environment-file' for dependency_management…

586adf7

… script. - And remove equivalent function from validate_consistency script. - Update pre-commit-hooks to reflect change.

Implement validation of rtd_reqs in dependency_management script.

c27d2bb

Implement validate-pyproject-toml in dependency_management script.

a65a0c8

Use pathlib for paths in dependency_management script.

78a3456

Use consistent exception type for dependency specification errors.

f3b5702

Place loading of 'setup.json' file in function.

d9beace

Fix bug in 'validate-environment-yml' command implementation.

0f5a60e

csadorf force-pushed the misc/dependency-management branch from 89a8b3e to 0f0899a Compare March 9, 2020 16:23

csadorf added the topic/dependencies label Mar 9, 2020

ltalirz suggested changes Mar 9, 2020

View reviewed changes

csadorf added 6 commits March 9, 2020 18:40

Only store requirements for one backend version (django).

519908c

Since the environments only differ across python versions, not backends.

Remove marker for numpy requirement.

0c5d2dc

Run the two extra workflows on the PR branch for testing.

1bb8351

Pin the Click version to 7.0.

bd2c172

The recent update to Click (7.1) breaks our tests and this CI workflow, because the help output formatting was slightly changed. Until we have resolved other issues we should pin the Click version to 7.0.

Slightly more explicit step names in test-install workflow.

33ac037

Fix typo in 'test-install' workflow definition.

51ecd99

csadorf force-pushed the misc/dependency-management branch from 1011093 to 51ecd99 Compare March 9, 2020 17:41

Merge branch 'develop' into misc/dependency-management

8666471

ltalirz self-requested a review March 9, 2020 17:42

ltalirz previously approved these changes Mar 9, 2020

View reviewed changes

csadorf self-assigned this Mar 9, 2020

Revert "Run the two extra workflows on the PR branch for testing."

75f609a

This reverts commit 1bb8351.

csadorf dismissed ltalirz’s stale review via 75f609a March 9, 2020 18:04

ltalirz self-requested a review March 9, 2020 18:54

ltalirz approved these changes Mar 9, 2020

View reviewed changes

ltalirz merged commit 7e99c06 into develop Mar 9, 2020

ltalirz mentioned this pull request Mar 9, 2020

Update requirements.txt #3833

Closed

sphuber deleted the misc/dependency-management branch March 27, 2020 11:12

csadorf mentioned this pull request Apr 20, 2020

Require spglib~=1.15 #3944

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update dependency management workflow #3771

Update dependency management workflow #3771

csadorf commented Feb 19, 2020

csadorf commented Feb 20, 2020

ltalirz commented Feb 20, 2020 •

edited

Loading

csadorf commented Feb 20, 2020

ltalirz left a comment

ltalirz left a comment •

edited

Loading

sphuber commented Mar 3, 2020

csadorf commented Mar 3, 2020

sphuber commented Mar 3, 2020

csadorf commented Mar 3, 2020

codecov-io commented Mar 4, 2020 •

edited by codecov bot

Loading

csadorf commented Mar 9, 2020 •

edited

Loading

ltalirz left a comment

ltalirz Mar 9, 2020

ltalirz left a comment

csadorf commented Mar 9, 2020

csadorf commented Mar 9, 2020

csadorf commented Mar 9, 2020

ltalirz commented Mar 9, 2020

ltalirz commented Mar 9, 2020

ltalirz commented Mar 9, 2020

csadorf commented Mar 9, 2020

	- '.github/workflows/test-install.sh'
	- '.github/workflows/test-install.yml'

Update dependency management workflow #3771

Update dependency management workflow #3771

Conversation

csadorf commented Feb 19, 2020

csadorf commented Feb 20, 2020

ltalirz commented Feb 20, 2020 • edited Loading

csadorf commented Feb 20, 2020

ltalirz left a comment

Choose a reason for hiding this comment

ltalirz left a comment • edited Loading

Choose a reason for hiding this comment

sphuber commented Mar 3, 2020

csadorf commented Mar 3, 2020

sphuber commented Mar 3, 2020

csadorf commented Mar 3, 2020

codecov-io commented Mar 4, 2020 • edited by codecov bot Loading

Codecov Report

csadorf commented Mar 9, 2020 • edited Loading

ltalirz left a comment

Choose a reason for hiding this comment

ltalirz Mar 9, 2020

Choose a reason for hiding this comment

ltalirz left a comment

Choose a reason for hiding this comment

csadorf commented Mar 9, 2020

csadorf commented Mar 9, 2020

csadorf commented Mar 9, 2020

ltalirz commented Mar 9, 2020

ltalirz commented Mar 9, 2020

ltalirz commented Mar 9, 2020

csadorf commented Mar 9, 2020

ltalirz commented Feb 20, 2020 •

edited

Loading

ltalirz left a comment •

edited

Loading

codecov-io commented Mar 4, 2020 •

edited by codecov bot

Loading

csadorf commented Mar 9, 2020 •

edited

Loading