GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — chore: Install python dependencies with uv in workflows by tokoko · Pull Request #4086 · feast-dev/feast

Conversation

@tokoko

@tokoko tokoko commented Apr 9, 2024

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:

  • Upgrades setup-python action to latest
  • Disables caching, caches are bound to branches and unit tests run on different PR source branches cannot reuse each other's cache. Only multiple runs on the same PR can.
  • Installs python dependencies with uv

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
tokoko added 4 commits April 9, 2024 17:47
@tokoko
install dependencies in unit-tests with uv
6e165e0
Signed-off-by: tokoko <[email protected]>
@tokoko
install dependencies in unit-tests with uv
8ea1210
Signed-off-by: tokoko <[email protected]>
@tokoko
enable caching, change linter job
2ba3708
Signed-off-by: tokoko <[email protected]>
@tokoko
change local integration tests to uv
e6400ac
Signed-off-by: tokoko <[email protected]>
@jeremyary jeremyary added the ok-to-test label Apr 9, 2024
@tokoko tokoko changed the title chore: Speed up unit tests in workflows chore: Install python dependencies with uv in workflows Apr 9, 2024
@tokoko
change all installs to uv
8f3b362
Signed-off-by: tokoko <[email protected]>
@tokoko tokoko mentioned this pull request Apr 10, 2024
Fix caching in Github Workflows #4056
Closed
@tmihalac

tmihalac commented Apr 10, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor
  • Disables caching, caches are bound to branches and unit tests run on different PR source branches cannot reuse each other's cache. Only multiple runs on the same PR can.

Maybe this helps somehow with the caches:
Have a look here in the documentation Cache not found for input keys

"The cache action first searches for cache hits for key and the cache version in the branch containing the workflow run. If there is no hit, it searches for restore-keys and the version. If there are still no hits in the current branch, the cache action retries same steps on the default branch"

I think it solves the problem with the caches because in a pull request the merging is usually into master in which the artifacts should already be in, don't you think ?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tmihalac
tmihalac reviewed Apr 10, 2024
View reviewed changes
Comment thread .github/workflows/nightly-ci.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

@tokoko

tokoko commented Apr 10, 2024

Copy link
Copy Markdown
Collaborator Author

@tmihalac that might be a good idea, unit tests are never run on master, but integration tests are and unit test jobs might be able to reuse caches written by integration tests. Not sure what will happen when two jobs try to update the cache with the same key at the same time though.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tokoko added 2 commits April 10, 2024 18:27
@tokoko
try adding uv cache
55bb636
Signed-off-by: tokoko <[email protected]>
@tokoko
fix lambda cache step name
3e15126
Signed-off-by: tokoko <[email protected]>
tmihalac
tmihalac reviewed Apr 10, 2024
View reviewed changes
Comment thread .github/workflows/nightly-ci.yml Resolved

Uh oh!

There was an error while loading. Please reload this page.

@tokoko
reenable caches for uv
4563ace
Signed-off-by: tokoko <[email protected]>
@tokoko

tokoko commented Apr 11, 2024

Copy link
Copy Markdown
Collaborator Author

@tmihalac One concern I still have is that looks like some people's experience with caching and uv on github is that often it might actually slow things down (see here). I have reenabled caches for all workflows except the linter one. We can use that to compare the install runtimes and then make a decision whether we should leave caching in or not... does that sound okay?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tmihalac

tmihalac commented Apr 11, 2024

Copy link
Copy Markdown
Contributor

Sounds good to me.

To me it seems the tests take shorter time, do you think the same ?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tokoko
remove dangling cache step
36338c6
Signed-off-by: tokoko <[email protected]>
@tokoko

tokoko commented Apr 11, 2024

Copy link
Copy Markdown
Collaborator Author

I'm not too sure actually, just from a sample size of 1, Install Dependencies step seems to take about 30 seconds without a cache hit (https://github.com/feast-dev/feast/actions/runs/8652609326/job/23725916068?pr=4086). When there is a cache hit, the combined runtime of cache retrieval and install for 3.9 is 23 seconds and for 3.10 it is 32 seconds. On top of that there was an initial cache upload step (that's no longer visible now) for the first run which took somewhere around 45 seconds.

P.S. For some reason everything's slower on mac vms 😄

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tokoko

tokoko commented Apr 17, 2024

Copy link
Copy Markdown
Collaborator Author

@jeremyary I think this is safe to merge

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tokoko tokoko mentioned this pull request Apr 19, 2024
chore: Use pixi to lock python dependencies in a single command #4114
Merged
@franciscojavierarceo

franciscojavierarceo commented Apr 19, 2024

Copy link
Copy Markdown
Member

Seems like a big change... CC @HaoXuAI

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tokoko

tokoko commented Apr 20, 2024

Copy link
Copy Markdown
Collaborator Author

fwiw, even though uv is brand new, airflow has already switched to using it in their ci, so it's somewhat battle-tested already.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@haoxu0

haoxu0 commented Apr 20, 2024

Copy link
Copy Markdown
Collaborator

Is the cache totally not making any benefits for build?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tokoko

tokoko commented Apr 20, 2024

Copy link
Copy Markdown
Collaborator Author

@HaoXuAI sorry, PR description is misleading now, I have added cache back in to monitor that. tldr version is that, right now it's probably a net loss actually, we are using an older version of cache action, it's slow to upload and sometimes just fails to find the cache even if it exists. With upgraded cache action and pip, it would definitely be of value, no question about it. The question is whether that still is the case with uv instead of pip. uv is much much faster than pip and to me it looks like cache often brings no real benefit, in other words time spent on cache upload/download is roughly the same as a fresh uv install. The current version of this PR disables caching for a linter job only and we can use that to make a final decision later.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tokoko

tokoko commented Apr 20, 2024

Copy link
Copy Markdown
Collaborator Author

This run is a good example. cache step seems to work at first, but decides the cache is invalid for some reason. pip install step takes 2.5 minutes without cache and then the job proceeds to waste almost 6 minutes to upload the directory again. For comparison uv install (without cache) is basically always under a minute.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

haoxu0
haoxu0 reviewed Apr 23, 2024
View reviewed changes
Comment thread Makefile
pip install --no-deps -e .
python setup.py build_python_protos --inplace

install-python-ci-dependencies-uv:

@haoxu0 haoxu0 Apr 23, 2024

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

i'm not familiar with uv. but using it to install in the CI, will it have a different result with direct pip install?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@tokoko tokoko Apr 23, 2024

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

For all intents and purposes, no, it's intended to be a drop-in replacement. There's of course some minor differences, pip tends to be more lenient towards packages with invalid metadata for example, version resolution (which we aren't changing in this pr, but I'll do in a follow-up) might also be negligibly different because they are using different resolvers with different heuristics. But at the end of the day, it shouldn't matter for us.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
haoxu0
haoxu0 approved these changes Apr 23, 2024
View reviewed changes

@haoxu0 haoxu0 left a comment

Copy link
Copy Markdown
Collaborator

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM.
like the idea of upgrading the python env tool.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@haoxu0
haoxu0 merged commit 6ef7852 into feast-dev:master Apr 23, 2024
@tokoko
tokoko deleted the ci-speedup branch April 25, 2024 09:05
lokeshrangineni pushed a commit to lokeshrangineni/feast that referenced this pull request Apr 25, 2024
@tokoko @lokeshrangineni
chore: Install python dependencies with uv in workflows (feast-dev#4086)
72f1e1b
* install dependencies in unit-tests with uv

Signed-off-by: tokoko <[email protected]>

* install dependencies in unit-tests with uv

Signed-off-by: tokoko <[email protected]>

* enable caching, change linter job

Signed-off-by: tokoko <[email protected]>

* change local integration tests to uv

Signed-off-by: tokoko <[email protected]>

* change all installs to uv

Signed-off-by: tokoko <[email protected]>

* try adding uv cache

Signed-off-by: tokoko <[email protected]>

* fix lambda cache step name

Signed-off-by: tokoko <[email protected]>

* reenable caches for uv

Signed-off-by: tokoko <[email protected]>

* remove dangling cache step

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>
jeremyary added a commit that referenced this pull request Apr 25, 2024
@lokeshrangineni @tokoko
@jeremyary @feast-ci-bot @pushkarmoi @tmihalac @TomSteenbergen
fix: Changed the code the way mysql container is initialized. (#4140)
8b5698f
* chore: Move feast install to docker build in java it tests (#4126)

* chore: Move feast install to docker build in java it tests

Signed-off-by: tokoko <[email protected]>

* remove commented out lines in compose file

Signed-off-by: tokoko <[email protected]>

* make local compose mode default

Signed-off-by: tokoko <[email protected]>

* limit COPY contents

Signed-off-by: tokoko <[email protected]>

* remove requirements.txt from java tests docker image

Signed-off-by: tokoko <[email protected]>

* include pyproject.toml in dockerfile

Signed-off-by: tokoko <[email protected]>

* change links to depends_on

Signed-off-by: tokoko <[email protected]>

* try updating setup-python to v5

Signed-off-by: tokoko <[email protected]>

* pin macos image to macos-12

Signed-off-by: tokoko <[email protected]>

* force rerun

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* chore: Incorporate release 0.37.1 to master (#4127)

* chore(release): release 0.37.1

## [0.37.1](v0.37.0...v0.37.1) (2024-04-17)

### Bug Fixes

* Pgvector patch ([#4108](#4108)) ([1a1f0b1](1a1f0b1))

### Reverts

* Reverts "fix: Using version args to install the correct feast version" ([#4112](#4112)) ([d5ded69](d5ded69)), closes [#3953](#3953)

* chore: Move feast install to docker build in java it tests (#4126)

* chore: Move feast install to docker build in java it tests

Signed-off-by: tokoko <[email protected]>

* remove commented out lines in compose file

Signed-off-by: tokoko <[email protected]>

* make local compose mode default

Signed-off-by: tokoko <[email protected]>

* limit COPY contents

Signed-off-by: tokoko <[email protected]>

* remove requirements.txt from java tests docker image

Signed-off-by: tokoko <[email protected]>

* include pyproject.toml in dockerfile

Signed-off-by: tokoko <[email protected]>

* change links to depends_on

Signed-off-by: tokoko <[email protected]>

* try updating setup-python to v5

Signed-off-by: tokoko <[email protected]>

* pin macos image to macos-12

Signed-off-by: tokoko <[email protected]>

* force rerun

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Co-authored-by: feast-ci-bot <[email protected]>
Co-authored-by: Tornike Gurgenidze <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* chore: Install python dependencies with uv in workflows (#4086)

* install dependencies in unit-tests with uv

Signed-off-by: tokoko <[email protected]>

* install dependencies in unit-tests with uv

Signed-off-by: tokoko <[email protected]>

* enable caching, change linter job

Signed-off-by: tokoko <[email protected]>

* change local integration tests to uv

Signed-off-by: tokoko <[email protected]>

* change all installs to uv

Signed-off-by: tokoko <[email protected]>

* try adding uv cache

Signed-off-by: tokoko <[email protected]>

* fix lambda cache step name

Signed-off-by: tokoko <[email protected]>

* reenable caches for uv

Signed-off-by: tokoko <[email protected]>

* remove dangling cache step

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* feat: Make arrow primary interchange for online ODFV execution (#4143)

* rewrite online flow to use transform_arrow

Signed-off-by: tokoko <[email protected]>

* fix transformation server

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* feat: Feast/IKV online store documentation (#4146)

* feat: Feast/IKV online store documentation

Signed-off-by: Pushkar Gupta <[email protected]>

* functionality matric

Signed-off-by: Pushkar Gupta <[email protected]>

* more changes

Signed-off-by: Pushkar Gupta <[email protected]>

* mount dir

Signed-off-by: Pushkar Gupta <[email protected]>

---------

Signed-off-by: Pushkar Gupta <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* fix: Default value is not set in Redis connection string using environment variable (#4136)

Removed documentation of Redis connection string supporting default values when using environment variables as it isn't supported

Fixes #3669

Signed-off-by: Theodor Mihalache <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* fix: Make sure schema is used when calling `get_table_query_string` method for Snowflake datasource (#4131)

* Fix get_table_query_string method for Snowflake datasource

Signed-off-by: TomSteenbergen <[email protected]>

* Add quotes to table string

Signed-off-by: TomSteenbergen <[email protected]>

---------

Signed-off-by: TomSteenbergen <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* fix: Change checkout action back to v3 from v5 which isn't released yet (#4147)

Signed-off-by: Jeremy Ary <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* changed the code the way mysql container is initialized. Trying to fix the issue - #4128
Also going to check if this change will be resolved in the github actions as well.

Signed-off-by: Lokesh Rangineni <[email protected]>

* reformatted the file to resolve lint error.

Signed-off-by: Lokesh Rangineni <[email protected]>

* reformatted the file to resolve lint error.

Signed-off-by: Lokesh Rangineni <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>
Signed-off-by: Pushkar Gupta <[email protected]>
Signed-off-by: Theodor Mihalache <[email protected]>
Signed-off-by: TomSteenbergen <[email protected]>
Signed-off-by: Jeremy Ary <[email protected]>
Co-authored-by: Tornike Gurgenidze <[email protected]>
Co-authored-by: Jeremy Ary <[email protected]>
Co-authored-by: feast-ci-bot <[email protected]>
Co-authored-by: Pushkar Gupta <[email protected]>
Co-authored-by: Theodor Mihalache <[email protected]>
Co-authored-by: Tom Steenbergen <[email protected]>
lokeshrangineni added a commit to lokeshrangineni/feast that referenced this pull request Apr 25, 2024
@lokeshrangineni @tokoko
@jeremyary @feast-ci-bot @pushkarmoi @tmihalac @TomSteenbergen
fix: Changed the code the way mysql container is initialized. (feast-…
d089b0b
…dev#4140)

* chore: Move feast install to docker build in java it tests (feast-dev#4126)

* chore: Move feast install to docker build in java it tests

Signed-off-by: tokoko <[email protected]>

* remove commented out lines in compose file

Signed-off-by: tokoko <[email protected]>

* make local compose mode default

Signed-off-by: tokoko <[email protected]>

* limit COPY contents

Signed-off-by: tokoko <[email protected]>

* remove requirements.txt from java tests docker image

Signed-off-by: tokoko <[email protected]>

* include pyproject.toml in dockerfile

Signed-off-by: tokoko <[email protected]>

* change links to depends_on

Signed-off-by: tokoko <[email protected]>

* try updating setup-python to v5

Signed-off-by: tokoko <[email protected]>

* pin macos image to macos-12

Signed-off-by: tokoko <[email protected]>

* force rerun

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* chore: Incorporate release 0.37.1 to master (feast-dev#4127)

* chore(release): release 0.37.1

## [0.37.1](feast-dev/feast@v0.37.0...v0.37.1) (2024-04-17)

### Bug Fixes

* Pgvector patch ([feast-dev#4108](feast-dev#4108)) ([1a1f0b1](feast-dev@1a1f0b1))

### Reverts

* Reverts "fix: Using version args to install the correct feast version" ([feast-dev#4112](feast-dev#4112)) ([d5ded69](feast-dev@d5ded69)), closes [feast-dev#3953](feast-dev#3953)

* chore: Move feast install to docker build in java it tests (feast-dev#4126)

* chore: Move feast install to docker build in java it tests

Signed-off-by: tokoko <[email protected]>

* remove commented out lines in compose file

Signed-off-by: tokoko <[email protected]>

* make local compose mode default

Signed-off-by: tokoko <[email protected]>

* limit COPY contents

Signed-off-by: tokoko <[email protected]>

* remove requirements.txt from java tests docker image

Signed-off-by: tokoko <[email protected]>

* include pyproject.toml in dockerfile

Signed-off-by: tokoko <[email protected]>

* change links to depends_on

Signed-off-by: tokoko <[email protected]>

* try updating setup-python to v5

Signed-off-by: tokoko <[email protected]>

* pin macos image to macos-12

Signed-off-by: tokoko <[email protected]>

* force rerun

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Co-authored-by: feast-ci-bot <[email protected]>
Co-authored-by: Tornike Gurgenidze <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* chore: Install python dependencies with uv in workflows (feast-dev#4086)

* install dependencies in unit-tests with uv

Signed-off-by: tokoko <[email protected]>

* install dependencies in unit-tests with uv

Signed-off-by: tokoko <[email protected]>

* enable caching, change linter job

Signed-off-by: tokoko <[email protected]>

* change local integration tests to uv

Signed-off-by: tokoko <[email protected]>

* change all installs to uv

Signed-off-by: tokoko <[email protected]>

* try adding uv cache

Signed-off-by: tokoko <[email protected]>

* fix lambda cache step name

Signed-off-by: tokoko <[email protected]>

* reenable caches for uv

Signed-off-by: tokoko <[email protected]>

* remove dangling cache step

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* feat: Make arrow primary interchange for online ODFV execution (feast-dev#4143)

* rewrite online flow to use transform_arrow

Signed-off-by: tokoko <[email protected]>

* fix transformation server

Signed-off-by: tokoko <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* feat: Feast/IKV online store documentation (feast-dev#4146)

* feat: Feast/IKV online store documentation

Signed-off-by: Pushkar Gupta <[email protected]>

* functionality matric

Signed-off-by: Pushkar Gupta <[email protected]>

* more changes

Signed-off-by: Pushkar Gupta <[email protected]>

* mount dir

Signed-off-by: Pushkar Gupta <[email protected]>

---------

Signed-off-by: Pushkar Gupta <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* fix: Default value is not set in Redis connection string using environment variable (feast-dev#4136)

Removed documentation of Redis connection string supporting default values when using environment variables as it isn't supported

Fixes feast-dev#3669

Signed-off-by: Theodor Mihalache <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* fix: Make sure schema is used when calling `get_table_query_string` method for Snowflake datasource (feast-dev#4131)

* Fix get_table_query_string method for Snowflake datasource

Signed-off-by: TomSteenbergen <[email protected]>

* Add quotes to table string

Signed-off-by: TomSteenbergen <[email protected]>

---------

Signed-off-by: TomSteenbergen <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* fix: Change checkout action back to v3 from v5 which isn't released yet (feast-dev#4147)

Signed-off-by: Jeremy Ary <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>

* changed the code the way mysql container is initialized. Trying to fix the issue - feast-dev#4128
Also going to check if this change will be resolved in the github actions as well.

Signed-off-by: Lokesh Rangineni <[email protected]>

* reformatted the file to resolve lint error.

Signed-off-by: Lokesh Rangineni <[email protected]>

* reformatted the file to resolve lint error.

Signed-off-by: Lokesh Rangineni <[email protected]>

---------

Signed-off-by: tokoko <[email protected]>
Signed-off-by: Lokesh Rangineni <[email protected]>
Signed-off-by: Pushkar Gupta <[email protected]>
Signed-off-by: Theodor Mihalache <[email protected]>
Signed-off-by: TomSteenbergen <[email protected]>
Signed-off-by: Jeremy Ary <[email protected]>
Co-authored-by: Tornike Gurgenidze <[email protected]>
Co-authored-by: Jeremy Ary <[email protected]>
Co-authored-by: feast-ci-bot <[email protected]>
Co-authored-by: Pushkar Gupta <[email protected]>
Co-authored-by: Theodor Mihalache <[email protected]>
Co-authored-by: Tom Steenbergen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@haoxu0 haoxu0 haoxu0 approved these changes
+1 more reviewer
@tmihalac tmihalac tmihalac left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@tokoko @tmihalac @franciscojavierarceo @haoxu0 @jeremyary
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.