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

Update all python dependencies #1213

Merged
merged 8 commits into from
Jun 8, 2024
Merged

Update all python dependencies #1213

merged 8 commits into from
Jun 8, 2024

Conversation

rogerluan
Copy link
Member

@rogerluan rogerluan commented Sep 18, 2023

Description

This PR updates all python dependencies we have by running pipenv update.

It requires us to move away from:

google_analytics:
- UA-18658848-13
- docs.fastlane.tools

to:

theme:
  analytics:
    gtag: G-111111111

Related PR: mkdocs/mkdocs#2253

We need to update to the right gtag value (GA4 property) which can only be fetched from fastlane's webiste analytics.google.com dashboard.

Until we update it, we'll see this:

image

References

This PR supersedes:

@rogerluan rogerluan added dependencies Pull requests that update a dependency file python Pull requests that update Python code labels Sep 18, 2023
@janbrasna
Copy link
Contributor

It requires us to move away from: […]

We need to update to the right gtag value (GA4 property) which can only be fetched from fastlane's webiste analytics.google.com dashboard.

@rogerluan If that makes any easier the old UA stopped gathering data in mid-2023 and by mid-2024 they won't even be accessible anymore for historic purposes, so if you can't get ahold of the new GA4 value you can just shim it or disable the analytics for now — as it won't have any impact on collecting analytics — they are NOT being collected already as we speak…

@janbrasna
Copy link
Contributor

BTW the fastlane.tools web last updated in 2022 also doesn't have any G–* tag GA4 ID so that isn't gathering any stats right now as well. (=Which I understand that there's no GA4 setup at all, so no ID to wait for.)

Since there's no cookie policy nag bar at docs.* and the GA would need to run in anonymous mode otherwise without that tracking consent, I'd say it's safe to completely disable the GA plugin at this time by not providing any value.

@rogerluan
Copy link
Member Author

This is super helpful @janbrasna 😲 I think you're right. We wouldn't be causing any regressions 👀 might end up going with that solution after all!

@rogerluan
Copy link
Member Author

It seems like RubyGems is undergoing maintenance and their servers are timing out… will check this later

@fastlane-bot-helper
Copy link
Collaborator

1 Warning
⚠️ mkdocs.yml was modified - make sure no modification was made by mistake

Generated by 🚫 Danger

@rogerluan
Copy link
Member Author

Looks like CI passed 🙏

Copy link
Collaborator

@revolter revolter left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@rogerluan rogerluan merged commit 8571a4d into master Jun 8, 2024
6 checks passed
@rogerluan rogerluan deleted the roger/update-dependencies branch June 8, 2024 19:43
@janbrasna
Copy link
Contributor

Dependabot is not smart enough to pick up the changes and close the obsolete PRs:

This PR supersedes:

so if anyone can do the honours and close them out? Thanks 💋

(For this reason I'd usually use closing keywords in the PR descriptions to explicitly make sure they get closed upon merging, even if the bot leaves them dangling… 🤦)

@revolter
Copy link
Collaborator

revolter commented Jun 9, 2024

Good catch, thanks! ❤️


(For this reason I'd usually use closing keywords in the PR descriptions to explicitly make sure they get closed upon merging, even if the bot leaves them dangling… 🤦)

Are you sure that you can auto close other PRs (not issues) from a PR? 🤔 I know that it can auto close issues: https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue.

@rogerluan
Copy link
Member Author

Yeah it works with PRs too :) I've used this before IIRC

@rogerluan
Copy link
Member Author

Dependabot could auto close those open PRs (it is smart enough actually! 😝), but I think that just happens when it runs again (which, for this repo, I think that happens once a month only 😁) — good call!

@revolter
Copy link
Collaborator

revolter commented Jun 9, 2024

Because we didn't interact with it lately, it was paused (https://github.blog/changelog/2023-01-12-dependabot-pull-requests-pause-for-inactivity/), and that's why it didn't close them.

@janbrasna
Copy link
Contributor

@revolter Yup, it may not necessarily be designed to, or documented for that matter, but works:

Screenshot 2024-06-09 at 15 00 04

Screenshot 2024-06-09 at 15 01 55

@rogerluan I thought so and checked before posting for dependency schedule settings to save unnecessary notifications… but that's probably somewhere in repo settings as there's no dependabot yml config file around here. Anyways I think it would take some time and maybe minor mkdocs bumps for it to realise the "*" is currently already satisfied by the installed version anyways, and close out the old PRs. (It's good for superseding old ones when newer are available, but not sure how eagerly it understands changes in (un)pinning versions et al. when it comes to just closing the old ones, esp. when there are conflicts pending to be resolved on its branches, better to do it manually FWIW…)

@janbrasna
Copy link
Contributor

PS: can't tell whether that's related or not, but the build step now takes not 5-15mins as before, not 25-30 like here in the PR, but master and anything against master now builds in 55-60mins FYI…

(I would understand that if the PR did add "ruby" as a platform to compile the stuff instead of PCG, but it actually removes it so any impact here escapes me…)

It's the bundle update step https://app.circleci.com/pipelines/github/fastlane/docs/869/workflows/46b17f7a-2939-48e6-bcb2-ac61e5f45892/jobs/3403/parallel-runs/0/steps/0-105?invite=true#step-105-0_45 spending all the time "Fetching https://github.com/fastlane/fastlane...... Fetching gem metadata from https://rubygems.org/......... Resolving dependencies......." so it may be some networking woes, but I don't see any service disruption notices anywhere.

@janbrasna
Copy link
Contributor

@revolter @rogerluan Anecdotally confirmed using various bundler versions, and extracting one bit from #1237 that did wonders TL;DR: I've opened #1249 that should fix this.

@janbrasna
Copy link
Contributor

janbrasna commented Jun 10, 2024

PS: I see the tests are passing, but looking at the logs ("Something went wrong - I couldn't find it...") I'm not 100% sure there isn't anything just skipping over it somehow also given how quick it is now (:) would you please doublecheck the steps/lanes output that all's dandy?

pipenv run bundle exec fastlane test log results: (click to expand)

18:00:50: --- Step: Switch to ensure_code_samples lane ---
18:00:50: ------------------------------------------------
18:00:50: Cruising over to lane 'ensure_code_samples' 🚖
18:00:50: 🕗 Verifying all code samples work with the latest fastlane release
tput: No value for $TERM and no -T specified
tput: No value for $TERM and no -T specified
[18:00:51]: ------------------------------
[18:00:51]: --- Step: test_sample_code ---
[18:00:51]: ------------------------------
(eval):1: warning: already initialized constant Fastlane::Actions::TestSampleCodeAction::ALL_LANGUAGES
(eval):1: warning: previous definition of ALL_LANGUAGES was here
Hi there
name
[18:00:51]: Something went wrong - I couldn't find it...
Hi Human!
Hi there
Hi there
Using Version 1.0
I can speak
(eval):1: warning: already initialized constant Fastlane::Actions::TestSampleCodeAction::ALL_LANGUAGES
(eval):1: warning: previous definition of ALL_LANGUAGES was here
[18:00:51]: ✅ All fastlane code samples (from 245 files) work as expected
[18:00:51]: Cruising back to lane 'test' 🚘

+-----------------------------------------------------------------+
| fastlane summary |
+------+--------------------------------------------+-------------+
| Step | Action | Time (in s) |
+------+--------------------------------------------+-------------+
| 1 | opt_out_usage | 0 |
| 2 | Switch to build_site lane | 0 |
| 3 | mkdocs build --strict | 17 |
| 4 | Switch to ensure_filenames lane | 0 |
| 5 | Switch to ensure_redirects lane | 0 |
| 6 | Switch to ensure_image_assets lane | 0 |
| 7 | Switch to ensure_assets_location lane | 0 |
| 8 | Switch to ensure_tool_name_formatting lane | 0 |
| 9 | Switch to ensure_code_samples lane | 0 |
| 10 | test_sample_code | 0 |
+------+--------------------------------------------+-------------+

[18:00:51]: fastlane.tools finished successfully 🎉

@revolter
Copy link
Collaborator

@janbrasna,

but looking at the logs ("Something went wrong - I couldn't find it...")

The test_sample_code custom action extracts the content from all the Markdown code blocks, and literally executes them using eval.

And, the error log that we're seeing in the output of the test lane is coming from one of the github_api action's example, which actually communicates with GitHub by making a GET request to https://api.github.com/repos/:owner/:repo/readme, which is obviously invalid.

I'm not 100% sure there isn't anything just skipping over it somehow also given how quick it is now

I see that the pipenv run bundle exec fastlane test step took 21 seconds, which looks alright to me. I can also see expected output there, which matches the output when I run it locally.

Looking at an older CI run, it looks like the website building was taking ~4 minutes, while it now takes ~17 seconds.

Since these changes already got deployed:

image

and the actual website seems to work alright, I can only assume that the mkdocs update from 1.1.2 to 1.6.0 greatly improved the performance 🤷🏻‍♂️

@janbrasna
Copy link
Contributor

Thanks for double checking 💟

With the perf improvements perhaps the mkdocs serve note is also no longer needed?

@revolter
Copy link
Collaborator

perhaps the mkdocs serve note is also no longer needed?

Looks like so:

INFO - Documentation built in 6.58 seconds

(output from running locally on my MacBook)

Opened #1255 with this removal, thanks for catching it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file python Pull requests that update Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants