New main update version and sources module. #1027

viniciusdc · 2020-06-01T20:26:23Z

As mentioned by @CJ-Wright and @beckermr this new PR aims some alterations on the original version of
update_upstream_versions code to generate a JSON file containing the required new versions outside the graph structure. Is also, now communicates with the others functions of that module.

Now, I've created a separate module for the upstream sources in the library. A test in a separate repo using Circle CI, was made with an good result (The code runs) Circle Ci.
There are still some missing alterations as:

Some sources still do some alterations directly to the graph structure;
As cited by CJ, I still need to implement it on the cli.xsh file to be read (and execute).
There is a missing feature, saliented by CJ that I (unfortunately) still could not set:

"Changes to make graph (or maybe the version bump code) to look at the source of new version numbers. (I think this is the main piece missing from this PR)"

So these are the next steps. (I would also like to apology myself for the late PR).

codecov · 2020-06-01T20:34:45Z

Codecov Report

Merging #1027 into master will decrease coverage by 4.94%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master    #1027      +/-   ##
==========================================
- Coverage   62.95%   58.00%   -4.95%     
==========================================
  Files          51       53       +2     
  Lines        4603     5025     +422     
==========================================
+ Hits         2898     2915      +17     
- Misses       1705     2110     +405

Impacted Files	Coverage Δ
conda_forge_tick/new_update_versions.py	`0.00% <0.00%> (ø)`
conda_forge_tick/update_sources.py	`0.00% <0.00%> (ø)`
conda_forge_tick/all_feedstocks.py	`20.33% <0.00%> (-4.11%)`	⬇️
conda_forge_tick/make_graph.py	`45.45% <0.00%> (-1.15%)`	⬇️
conda_forge_tick/migrators/disabled/legacy.py	`68.82% <0.00%> (-0.85%)`	⬇️
tests/test_migrators.py	`93.88% <0.00%> (-0.04%)`	⬇️
tests/test_audit.py	`100.00% <0.00%> (ø)`
tests/test_use_pip.py	`100.00% <0.00%> (ø)`
tests/test_patch_dict.py	`100.00% <0.00%> (ø)`
conda_forge_tick/utils.py	`62.16% <0.00%> (ø)`
... and 27 more

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 e34cec4...8892b40. Read the comment docs.

CJ-Wright · 2020-06-01T22:37:08Z

We could push 3 into make_graph itself, having it draw from the new versions file and inserting them into the nodes. This would allow us to make the least changes to the rest of the code.

viniciusdc · 2020-06-02T14:44:11Z

We could push 3 into make_graph itself, having it draw from the new versions file and inserting them into the nodes. This would allow us to make the least changes to the rest of the code.

Yes, thanks for the advice. I'm right now looking into make-graph script and function. I will look camly to it, cause there are so many things running inside it.

viniciusdc · 2020-06-02T14:45:51Z

By the way, what means these tests of Travis and codecov ?

viniciusdc · 2020-06-02T16:19:23Z

We could push 3 into make_graph itself, having it draw from the new versions file and inserting them into the nodes. This would allow us to make the least changes to the rest of the code.

Hi again, I am not sure about one thing, when the bot is running it first executes the make_graph phase and them runs the update upstream versions phase, right ? So, if we add the versions from the output file inside the graph nodes, it will be not so updated right ?

viniciusdc · 2020-06-08T22:41:18Z

I'm testing the new additions, but I want to know if there is a way to save the output file created using the Circleci run ?

viniciusdc · 2020-06-09T03:03:50Z

I discovered how, but it's really troublesome work with the SSH keys. So I will not be able to test the make_graph function. I Am creating a new PR with the updates. (and closing this one)... (as soon as the test with the new JSON output at Circle finishess)

conda_forge_tick/new_update_versions.py

beckermr · 2020-06-09T17:24:24Z

conda_forge_tick/new_update_versions.py

+
+    logger.info("writing out file")
+    with open("new_version.json", "w") as outfile:
+        json.dump(to_update, outfile)


We should dump one file per feedstock.

Is there a specific path we should use for them ? (like bump on it's Node path)

Can I create a file on cf-graph-countyfair to save these files according to node ?(cf-graph-countyfair/versions/{node}_new_version.json)

if so, how can I send this file to that repo ?

You can make a folder using os.makedirs(foldername, exist_ok=True). The files should just be named {node}.json since the folder name should be descriptive that they are version numbers.

oh I see, them as we already have a Job on circle that saves all the alterations made during the running, them it will push the alterations automatically right ? If that's so I will made the corrections right away thanks 👍

Done, could someone check if it's ok ?

viniciusdc · 2020-06-10T05:12:41Z

I will change the new version file (outputs) and them close this PR. Them create a new one using the correct name as update_upstream_versions and together with make_graph.

CJ-Wright · 2020-06-11T02:09:29Z

I thought the updates to make_graph were going to go in an independent PR?

viniciusdc · 2020-06-11T02:15:48Z

I thought the updates to make_graph were going to go in an independent PR?

Sorry my bad, I will update this with the new name and removing the current update_upstream_versions. And them open a new PR with make_graph. But for that I just need to know how to save the output file (from circle) to cf-graph-county fair (cause I need to open it using the make_graph).

CJ-Wright · 2020-06-11T02:30:59Z

I would not remove update_upstream_versions in this PR.

In my opinion the ordering of changes should be:

Merge this PR with the new setup
Start the new setup on circle so that we actually populate the folder without disrupting current operations.
Once we're happy with the way the code is working make the API changes (changing make_graph to use the new folder's info and renaming the new update file to update_upstream_versions and removing the old one.

This ordering makes certain that we have a minimal amount of time where the system might not work and gives us multiple points to turn back or fix things without disrupting the bot. API changes usually bring all sorts of issues with them, even when we've reviewed the code a lot, so we try to keep API breaking PRs small. Ideally an API break PR only has the API break, no other pieces of code.

The deploy function makes certain that everything is properly added to git, committed and pushed. You will most likely need to add the folder with the new versions to that, which should go in this PR.

viniciusdc · 2020-06-11T02:39:27Z

Ok, thank you very much for the help. I will change the current way the files are being saved as stated by Matthew, and then merge. After that I will work into the circle job.

viniciusdc · 2020-06-11T02:40:23Z

Is that okay for me to create the new versions folder at cf-graph-countyfair ?

beckermr · 2020-06-12T12:13:54Z

Is that okay for me to create the new versions folder at cf-graph-countyfair ?

Yup!

A few things though

Never force push to this repo.
Never destroy files in it.
If you are unsure, ask.

This repo holds all of the core data that runs the bot, so we have to be careful!

CJ-Wright · 2020-06-12T12:40:13Z

I would rather that we use os.makedirs so we are certain the folder will exist, even in the event that it is deleted.

viniciusdc · 2020-06-12T16:36:53Z

I would rather that we use os.makedirs so we are certain the folder will exist, even in the event that it is deleted.

Thanks, I was going to use a os.path.isfile to check that. Good to know 👍

viniciusdc · 2020-06-12T16:39:25Z

Is that okay for me to create the new versions folder at cf-graph-countyfair ?

Yup!

A few things though

Never force push to this repo.

Never destroy files in it.

If you are unsure, ask.

This repo holds all of the core data that runs the bot, so we have to be careful!

Thanks for the advice. I've completed the changes and will commit them as soon as possible.
(The circle jobs that I can edit (or create) are at circle_workers or here ?)

beckermr · 2020-06-12T17:28:06Z

Those jobs are in the circle_worker repo

viniciusdc · 2020-06-12T18:48:43Z

Those jobs are in the circle_worker repo

Thanks, if the new dump structure is ok, I will work at this ^

viniciusdc · 2020-06-12T23:02:03Z

I've added the changes to cli.xsh as mentioned. For circle_worker this will come at another PR 61.

viniciusdc · 2020-06-14T23:13:17Z

May I merge this one ?

CJ-Wright · 2020-06-14T23:15:49Z

Please don't merge your own PRs (it is a bit of an anti-pattern)

viniciusdc · 2020-06-14T23:17:22Z

Ok sorry about that. Thanks for the advice 👍

viniciusdc · 2020-06-15T16:17:56Z

Is there something I need to change for the merge ? Is there any other thing I can do ?

conda_forge_tick/make_graph.py

CJ-Wright · 2020-06-15T22:25:08Z

Sorry for the review delay. Yes, if you are blocked on one PR/part of your project please feel free to start working on the next part. (assuming one doesn't depend on the other)

viniciusdc · 2020-06-16T01:21:22Z

Sorry for the review delay. Yes, if you are blocked on one PR/part of your project please feel free to start working on the next part. (assuming one doesn't depend on the other)

Actually I do need this running at circle to work on the make_graph PR, right ? So if there is any other part, that I maybe missing please inform me, and I will work into this 👍

viniciusdc · 2020-06-16T01:29:50Z

Then I will work on the pool version of the update_versions.

viniciusdc · 2020-06-16T18:04:43Z

@CJ-Wright I saw your [#1032], can I implement these name changes here too ?

CJ-Wright · 2020-06-16T18:36:34Z

If you want to do it on the new code you wrote, sure. Please don't change the code you haven't already changed though.

viniciusdc · 2020-06-16T18:42:09Z

If you want to do it on the new code you wrote, sure. Please don't change the code you haven't already changed though.

Yup, thanks.

conda_forge_tick/update_sources.py

CJ-Wright · 2020-06-17T02:39:05Z

this needs black run on it.

ocefpaf · 2020-06-17T15:23:14Z

this needs black run on it.

@CJ-Wright are you opened to add a pre-commit hook here?

marcelotrevisani · 2020-06-17T19:24:50Z

this needs black run on it.

@CJ-Wright are you opened to add a pre-commit hook here?

grayskull is already using it, if you want to you can copy from there or get that as example
https://github.com/marcelotrevisani/grayskull/blob/master/.pre-commit-config.yaml

viniciusdc · 2020-06-17T19:55:52Z

Sorry IDK what is a pre-commit in your sense of speaking, can someone clarify this to me ?

viniciusdc · 2020-06-17T19:58:40Z

Then I will work on the pool version of the update_versions.

I've just finished the pool version here, I will test here and them commit the changes. (one question can I put the two of them here as update...version sequential and update...version pool ?)

viniciusdc · 2020-06-17T20:01:15Z

There was any change with Travis ? Cause now Travis build passed successfully !

marcelotrevisani · 2020-06-17T20:30:17Z

Sorry IDK what is a pre-commit in your sense of speaking, can someone clarify this to me ?

Sorry, I should be more clear
pre-commit is a tool which leverages the git hooks to run linters, or some tools to improve code quality before you commit code

You can find more information about this tool here:
https://pre-commit.com/
https://github.com/pre-commit/pre-commit

viniciusdc · 2020-06-17T20:43:52Z

Sorry IDK what is a pre-commit in your sense of speaking, can someone clarify this to me ?

Sorry, I should be more clear
pre-commit is a tool which leverages the git hooks to run linters, or some tools to improve code quality before you commit code

You can find more information about this tool here:
https://pre-commit.com/
https://github.com/pre-commit/pre-commit

Thank you ! 🙂

CJ-Wright · 2020-06-17T22:25:58Z

LGTM

viniciusdc added 2 commits June 1, 2020 17:10

New update upstream versions main function

fe15617

Added the upstream sources as an independent module

3e97002

viniciusdc added 2 commits June 9, 2020 13:50

logger conda-forge-tick

659e17d

changes at the output file

fab2f14

beckermr reviewed Jun 9, 2020

View reviewed changes

conda_forge_tick/new_update_versions.py Outdated Show resolved Hide resolved

beckermr reviewed Jun 9, 2020

View reviewed changes

conda_forge_tick/new_update_versions.py Outdated Show resolved Hide resolved

beckermr reviewed Jun 9, 2020

View reviewed changes

Removed new_version_(attempts, errors)

d5df5ee

dump one JSON file per feedstock

641b2b9

'writing out file' message changed to debug

ea484c7

viniciusdc closed this Jun 12, 2020

added run call for new_update_versions

d9eb185

viniciusdc mentioned this pull request Jun 12, 2020

populate versions folder regro/circle_worker#61

Merged

CJ-Wright reviewed Jun 15, 2020

View reviewed changes

conda_forge_tick/make_graph.py Outdated Show resolved Hide resolved

rm update_nodes_with_new_version

541a4d2

CJ-Wright requested review from marcelotrevisani and ocefpaf June 16, 2020 17:28

marcelotrevisani reviewed Jun 16, 2020

View reviewed changes

conda_forge_tick/update_sources.py Show resolved Hide resolved

marcelotrevisani reviewed Jun 16, 2020

View reviewed changes

conda_forge_tick/update_sources.py Show resolved Hide resolved

reformatted minor cases

8892b40

CJ-Wright merged commit a1e09a1 into regro:master Jun 18, 2020

New main update version and sources module. #1027

New main update version and sources module. #1027

Conversation

viniciusdc commented Jun 1, 2020 • edited

codecov bot commented Jun 1, 2020 • edited

Codecov Report

CJ-Wright commented Jun 1, 2020

viniciusdc commented Jun 2, 2020

viniciusdc commented Jun 2, 2020

viniciusdc commented Jun 2, 2020

viniciusdc commented Jun 8, 2020

viniciusdc commented Jun 9, 2020 • edited

beckermr Jun 9, 2020

Choose a reason for hiding this comment

viniciusdc Jun 9, 2020

Choose a reason for hiding this comment

viniciusdc Jun 10, 2020 • edited

Choose a reason for hiding this comment

viniciusdc Jun 10, 2020

Choose a reason for hiding this comment

CJ-Wright Jun 11, 2020

Choose a reason for hiding this comment

viniciusdc Jun 11, 2020

Choose a reason for hiding this comment

viniciusdc Jun 12, 2020

Choose a reason for hiding this comment

viniciusdc commented Jun 10, 2020

CJ-Wright commented Jun 11, 2020

viniciusdc commented Jun 11, 2020

CJ-Wright commented Jun 11, 2020

viniciusdc commented Jun 11, 2020

viniciusdc commented Jun 11, 2020

beckermr commented Jun 12, 2020

CJ-Wright commented Jun 12, 2020

viniciusdc commented Jun 12, 2020

viniciusdc commented Jun 12, 2020 • edited

beckermr commented Jun 12, 2020

viniciusdc commented Jun 12, 2020

viniciusdc commented Jun 12, 2020 • edited

viniciusdc commented Jun 14, 2020

CJ-Wright commented Jun 14, 2020

viniciusdc commented Jun 14, 2020

viniciusdc commented Jun 15, 2020

CJ-Wright commented Jun 15, 2020 • edited

viniciusdc commented Jun 16, 2020

viniciusdc commented Jun 16, 2020

viniciusdc commented Jun 16, 2020

CJ-Wright commented Jun 16, 2020

viniciusdc commented Jun 16, 2020

CJ-Wright commented Jun 17, 2020

ocefpaf commented Jun 17, 2020

marcelotrevisani commented Jun 17, 2020

viniciusdc commented Jun 17, 2020 • edited

viniciusdc commented Jun 17, 2020 • edited

viniciusdc commented Jun 17, 2020

marcelotrevisani commented Jun 17, 2020 • edited

viniciusdc commented Jun 17, 2020

CJ-Wright commented Jun 17, 2020

viniciusdc commented Jun 1, 2020 •

edited

codecov bot commented Jun 1, 2020 •

edited

viniciusdc commented Jun 9, 2020 •

edited

viniciusdc Jun 10, 2020 •

edited

viniciusdc commented Jun 12, 2020 •

edited

viniciusdc commented Jun 12, 2020 •

edited

CJ-Wright commented Jun 15, 2020 •

edited

viniciusdc commented Jun 17, 2020 •

edited

viniciusdc commented Jun 17, 2020 •

edited

marcelotrevisani commented Jun 17, 2020 •

edited