feature/get-updated-legistar-matter #94

dphoria · 2022-04-23T22:52:28Z

Link to Relevant Issue

This pull request is a part of #59 .

Description of Changes

LegistarScraper.get_updated_model(matter) -> LegistarScraper.get_updated_model(matter) where Legistar API is queried for the given matter.

from cdp_backend.pipeline.ingestion_models import Matter
from cdp_scrapers.instances.seattle import SeattleScraper

old_matter = Matter(name="CB 120305", matter_type=None, title=None)
s = SeattleScraper()
s.get_updated_model(old_matter)
# Matter(name='CB 120305', matter_type='Council Bill (CB)', title='CB 120305', result_status='In Progress', sponsors=[Person(name='Dan Strauss', is_active=True, router_string=None, email='Dan.Strauss@seattle.gov', phone='206-684-8806', website='http://www.seattle.gov/council/strauss', picture_uri=None, seat=None, external_source_id='664')], external_source_id='12846')

s.get_updated_model(Matter(name=None, matter_type=None, title=None, external_source_id=1))
# None

Eventually, will provide API similar to get_{municipality}_events(), e.g.

from cdp_scrapers.instances import get_seattle_updated_model
get_seattle_updated_model(old_matter)

dphoria · 2022-04-23T22:54:57Z

/test-scraper get_king_county_events --from_dt=2022-04-04T00:00:00 --to_dt=2022-04-09T00:00:00

github-actions · 2022-04-23T22:55:07Z

Begin /test-scraper. This may take several minutes.

github-actions · 2022-04-23T22:56:08Z

/test-scraper is finished.
Please go to the Actions page then click on the latest workflow titled "feature/get-updated-legistar-matter".
The results are saved in scraper-test-results under Artifacts on the workflow summary page.

dphoria · 2022-04-23T22:56:12Z

/test-scraper get_king_county_events --from_dt=2022-04-04T00:00:00 --to_dt=2022-04-09T00:00:00

Just to prove nothing has broken. This PR yields no changes to scraped data. Have run and passed cdp_scrapers/test/scraper_tests.py.

dphoria · 2022-04-23T22:57:30Z

Before more work is committed, wanted to see if something like get_{municipality}_updated_model() will be a good API from cdp-backend perspective, @JacksonMaxfield @isaacna .

So just get_updated_model() in this PR. In fact will break up future PRs for this issue to keep them manageable.

dphoria · 2022-04-23T22:59:04Z

/test-scraper is finished. Please go to the Actions page then click on the latest workflow titled "feature/get-updated-legistar-matter". The results are saved in scraper-test-results under Artifacts on the workflow summary page.

https://github.com/CouncilDataProject/cdp-scrapers/actions/runs/2213727492

evamaxfield

Okay so I think I get what is going on and in general I like it.

A couple of comments / questions:

I saw a bit of the code that said like get_matter vs find_matter -- just to fully understand, the get matter takes and id where find matter takes the name?
There is a lot of fancy caching that is happening that could probably be abstracted away by using lru_cache see my use of it in cdp-data here.
we should hook up seattle-staging to this repo so that everytime a PR runs on this repo, it runs the pipeline for seattle. I am thinking of a ci job that wipes seattle staging DB and FS, then runs the pipeline with this scraper, then comments with a link(s) to the stored event. Would make it easier to "trust" this. (not that I don't trust you hahahah)

evamaxfield · 2022-05-04T20:39:52Z

cdp_scrapers/legistar_utils.py

+        if isinstance(model, Matter):
+            return self.get_updated_matter(model)


Because I assume we are going to have a lot of these, it may be better to have a LUT.

MODEL_UPDATER_LUT = { Matter: self.get_updated_matter, Person: self.get_updated_person, } return MODEL_UPDATER_LUT[type(model)]

evamaxfield · 2022-05-04T20:45:43Z

cdp_scrapers/legistar_utils.py

 LEGISTAR_MATTER_SPONSORS = "MatterSponsorInfo"
-LEGISTAR_SPONSOR_PERSON = "SponsorPersonInfo"


Dropped this?

Yes. So before, we never queried the full Legistar Matter. We just used bits of Legistar EventItem like EventItemMatterName to create ingestion model Matter.
Now that there is need to query for a Legistar Matter, I implemented get_legistar_matter() so I can reuse it from both the main get_legistar_events_for_timespan() and this new get_updated_model()...

So, before in get_legistar_events_for_timespan()

legistar_event_item = { ... "SponsorPersonInfo" : [ { "PersonFullName" : ... }, ... ], ... }

Now,

legistar_event_item = { ... "EventItemMatterInfo" : { "MatterFile" : ..., "MatterSponsorInfo" : [ { "PersonFullName" : ... }, ... ], ... }, ... }

evamaxfield · 2022-05-04T20:55:13Z

Other general comment is that I see you are using the ingestion model as the "update" model but since this is the "update API" it may be better to update from the database models and not the ingestion models.

isaacna · 2022-05-05T02:46:04Z

cdp_scrapers/legistar_utils.py

@@ -99,6 +99,8 @@

 LEGISTAR_EV_ITEMS = "EventItems"
 LEGISTAR_EV_ATTACHMENTS = "EventItemMatterAttachments"
+LEGISTAR_EV_MATTER_ID = "EventItemMatterId"


Just for clarification, what's the difference between LEGISTAR_EV_MATTER_ID and LEGISTAR_MATTER_EXT_ID? I'm not too familiar with Legistar's schema

Given the same Legistar Matter, the value will be the same.

LEGISTAR_EV_MATTER_ID = "EventItemMatterId" is a field in the Legistar Event Item. So, after querying for a Legistar Event Item, we can then query for the associated Matter using that field.

LEGISTAR_MATTER_EXT_ID = "MatterId" is a field in the Legistar Matter. After obtaining a Legistar Matter, we use that field to set Matter.external_source_id.

isaacna · 2022-05-05T02:55:44Z

cdp_scrapers/legistar_utils.py

+    matter: Dict[str, Any] = resp.json()
+
+    # Person JSON for this matter's sponsors
+    matter[LEGISTAR_MATTER_SPONSORS] = reduced_list(


I thought this was some built-in python method at first and was kinda confused lol. I think this list comprehension is a little awkward to read but I'm fine with it

That's a good point. I will change this. Readability is important for us.

isaacna · 2022-05-05T03:06:33Z

Eventually, will provide API similar to get_{municipality}_events(), e.g.

I'm personally a little more partial to something like get_events(municipality) instead of keeping separate methods for each instance. @JacksonMaxfield what do you think?

dphoria · 2022-05-05T03:09:14Z

1. I saw a bit of the code that said like `get_matter` vs `find_matter` -- just to fully understand, the get matter takes and id where find matter takes the name?

Correct. And I don't expect the find-by-name code path to be used that much, but I wrote it just in case we don't have the Legistar MatterId for the requested Matter.

2. There is a lot of fancy caching that is happening that could probably be abstracted away by using `lru_cache` see my use of it in `cdp-data` [here](https://github.com/CouncilDataProject/cdp-data/blob/main/cdp_data/utils/db_utils.py#L48).

Thank you!

3. we should hook up seattle-staging to this repo so that everytime a PR runs on this repo, it runs the pipeline for seattle. I am thinking of a ci job that wipes seattle staging DB and FS, then runs the pipeline with this scraper, then comments with a link(s) to the stored event. Would make it easier to "trust" this. (not that I don't trust you hahahah)

No no, definitely do not trust me LOL 🤣

dphoria · 2022-05-05T03:25:53Z

Other general comment is that I see you are using the ingestion model as the "update" model but since this is the "update API" it may be better to update from the database models and not the ingestion models.

Ah I like this.

evamaxfield · 2022-05-05T03:56:19Z

we should hook up seattle-staging to this repo so that everytime a PR runs on this repo, it runs the pipeline for seattle. I am thinking of a ci job that wipes seattle staging DB and FS, then runs the pipeline with this scraper, then comments with a link(s) to the stored event. Would make it easier to "trust" this. (not that I don't trust you hahahah)

I am pretty busy but that may be a good project for someone who wants to learn devops stuff. It also ties in well with: CouncilDataProject/cdp-backend#166

Further, I just think we could use it for all scrapers. So we have test scraper command but we should have a command that is like test-scraper --full or something?? similarly, if we can hook it up to the cookiecutter bot so that whatever scraper we generate, we can preview the deployment on staging before we fully deploy it 👀

evamaxfield · 2022-05-05T03:59:40Z

I'm personally a little more partial to something like get_events(municipality) instead of keeping separate methods for each instance. @JacksonMaxfield what do you think?

Those functions are auto generated. If all CDP instances were backed by legistar instances then that proposal would work but since not all are we shouldn't even try to guarentee that the API will be consistent across all of them. Hence why we generate those functions. If that doesn't make sense happy to expand more. I am sort of thinking that make sense but I could have talked myself into a circle.

evamaxfield · 2022-05-12T20:52:57Z

I am finally getting around to checking on other PRs. I don't think I need to do anything on this one right? No update yet? (no pressure I am just behind and lost track of stuff)

dphoria added 4 commits April 23, 2022 13:18

query Matter in get_legistar_events_for_timespan

27aec68

get_legistar_matter

55d5aeb

get_model

d2101d8

docstring

98079ef

dphoria added the enhancement New feature or request label Apr 23, 2022

dphoria marked this pull request as ready for review April 23, 2022 22:58

dphoria requested review from evamaxfield and isaacna April 23, 2022 22:58

dphoria added 7 commits April 23, 2022 16:15

handle request exception

65db202

typo

0e67853

poor name

e3681ed

inject_known_person, sanitize_roles

261442b

formatting

1aa9594

formatting

d7d17c4

formatting

3b75790

evamaxfield reviewed May 4, 2022

View reviewed changes

isaacna reviewed May 5, 2022

View reviewed changes

dphoria closed this Sep 17, 2022

dphoria deleted the feature/get-model branch September 17, 2022 16:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature/get-updated-legistar-matter #94

feature/get-updated-legistar-matter #94

dphoria commented Apr 23, 2022 •

edited

dphoria commented Apr 23, 2022

github-actions bot commented Apr 23, 2022

github-actions bot commented Apr 23, 2022

dphoria commented Apr 23, 2022

dphoria commented Apr 23, 2022 •

edited

dphoria commented Apr 23, 2022

evamaxfield left a comment

evamaxfield May 4, 2022

evamaxfield May 4, 2022

dphoria May 5, 2022

evamaxfield commented May 4, 2022

isaacna May 5, 2022 •

edited

dphoria May 5, 2022

isaacna May 5, 2022

dphoria May 5, 2022

isaacna commented May 5, 2022

dphoria commented May 5, 2022

dphoria commented May 5, 2022

evamaxfield commented May 5, 2022

evamaxfield commented May 5, 2022

evamaxfield commented May 12, 2022

		if isinstance(model, Matter):
		return self.get_updated_matter(model)

		LEGISTAR_MATTER_SPONSORS = "MatterSponsorInfo"
		LEGISTAR_SPONSOR_PERSON = "SponsorPersonInfo"

feature/get-updated-legistar-matter #94

feature/get-updated-legistar-matter #94

Conversation

dphoria commented Apr 23, 2022 • edited

Link to Relevant Issue

Description of Changes

dphoria commented Apr 23, 2022

github-actions bot commented Apr 23, 2022

github-actions bot commented Apr 23, 2022

dphoria commented Apr 23, 2022

dphoria commented Apr 23, 2022 • edited

dphoria commented Apr 23, 2022

evamaxfield left a comment

Choose a reason for hiding this comment

evamaxfield May 4, 2022

Choose a reason for hiding this comment

evamaxfield May 4, 2022

Choose a reason for hiding this comment

dphoria May 5, 2022

Choose a reason for hiding this comment

evamaxfield commented May 4, 2022

isaacna May 5, 2022 • edited

Choose a reason for hiding this comment

dphoria May 5, 2022

Choose a reason for hiding this comment

isaacna May 5, 2022

Choose a reason for hiding this comment

dphoria May 5, 2022

Choose a reason for hiding this comment

isaacna commented May 5, 2022

dphoria commented May 5, 2022

dphoria commented May 5, 2022

evamaxfield commented May 5, 2022

evamaxfield commented May 5, 2022

evamaxfield commented May 12, 2022

dphoria commented Apr 23, 2022 •

edited

dphoria commented Apr 23, 2022 •

edited

isaacna May 5, 2022 •

edited